Bug 192808

Summary: Use Ref<> as much as possible
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, darin, ggaren, rniwa, saam, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch achristensen: review+

Yusuke Suzuki
Reported 2018-12-18 00:56:36 PST
Use Ref<> as much as possible
Attachments
Patch (377.18 KB, patch)
2018-12-18 00:57 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (377.75 KB, patch)
2018-12-18 01:14 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (380.40 KB, patch)
2018-12-18 01:31 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (382.12 KB, patch)
2018-12-18 01:37 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (382.39 KB, patch)
2018-12-18 02:02 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (389.05 KB, patch)
2018-12-18 02:40 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (389.14 KB, patch)
2018-12-18 03:05 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (389.14 KB, patch)
2018-12-18 03:37 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (396.74 KB, patch)
2018-12-18 04:00 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (397.14 KB, patch)
2018-12-18 05:40 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (398.20 KB, patch)
2018-12-18 06:07 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (398.23 KB, patch)
2018-12-18 06:28 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (398.33 KB, patch)
2018-12-18 07:46 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (398.33 KB, patch)
2018-12-18 11:06 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (398.54 KB, patch)
2018-12-19 00:22 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (397.24 KB, patch)
2018-12-20 00:02 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (397.29 KB, patch)
2018-12-20 01:19 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (399.70 KB, patch)
2018-12-20 08:21 PST, Yusuke Suzuki 		achristensen: review+
DetailsFormatted DiffDiff
Show Obsolete (17) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2018-12-18 00:57:36 PST
Created attachment 357549 [details] Patch
Yusuke Suzuki
Comment 2 2018-12-18 01:14:18 PST
Created attachment 357552 [details] Patch
Yusuke Suzuki
Comment 3 2018-12-18 01:31:38 PST
Created attachment 357553 [details] Patch
Yusuke Suzuki
Comment 4 2018-12-18 01:37:36 PST
Created attachment 357554 [details] Patch
Yusuke Suzuki
Comment 5 2018-12-18 02:02:54 PST
Created attachment 357555 [details] Patch
Yusuke Suzuki
Comment 6 2018-12-18 02:40:46 PST
Created attachment 357556 [details] Patch
Yusuke Suzuki
Comment 7 2018-12-18 03:05:12 PST
Created attachment 357557 [details] Patch
Yusuke Suzuki
Comment 8 2018-12-18 03:37:17 PST
Created attachment 357560 [details] Patch
Yusuke Suzuki
Comment 9 2018-12-18 04:00:53 PST
Created attachment 357561 [details] Patch
Yusuke Suzuki
Comment 10 2018-12-18 05:40:15 PST
Created attachment 357562 [details] Patch
Yusuke Suzuki
Comment 11 2018-12-18 06:07:59 PST
Created attachment 357563 [details] Patch
Yusuke Suzuki
Comment 12 2018-12-18 06:28:58 PST
Created attachment 357564 [details] Patch
Yusuke Suzuki
Comment 13 2018-12-18 07:46:38 PST
Created attachment 357569 [details] Patch
Yusuke Suzuki
Comment 14 2018-12-18 11:06:27 PST
Created attachment 357584 [details] Patch
Saam Barati
Comment 15 2018-12-18 23:28:29 PST
Comment on attachment 357584 [details] Patch Seems like you have some build errors
Yusuke Suzuki
Comment 16 2018-12-19 00:22:47 PST
Created attachment 357665 [details] Patch
Yusuke Suzuki
Comment 17 2018-12-19 02:01:05 PST
OK, the patch is ready now :)
Alex Christensen
Comment 18 2018-12-19 13:05:54 PST
Comment on attachment 357665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357665&action=review > Source/JavaScriptCore/API/JSTypedArray.cpp:188 > + Ref<ArrayBuffer> buffer = ArrayBuffer::createFromBytes(bytes, length, [=](void* p) { auto buffer? > Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:59 > + Ref<ArrayBuffer> newBuffer = thisObject->impl()->slice(begin, end); auto newBuffer? > Source/WebCore/Modules/fetch/FetchBody.cpp:259 > + ASSERT(data); It seems like this whole function should be replaced by switchOn. Right now it is not obvious that this assertion is correct. Could we leave this one as a RefPtr for now?
Yusuke Suzuki
Comment 19 2018-12-20 00:02:54 PST
Created attachment 357796 [details] Patch
Yusuke Suzuki
Comment 20 2018-12-20 00:04:58 PST
Comment on attachment 357665 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=357665&action=review Thanks! >> Source/JavaScriptCore/API/JSTypedArray.cpp:188 >> + Ref<ArrayBuffer> buffer = ArrayBuffer::createFromBytes(bytes, length, [=](void* p) { > > auto buffer? Fixed. >> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:59 >> + Ref<ArrayBuffer> newBuffer = thisObject->impl()->slice(begin, end); > > auto newBuffer? Fixed. >> Source/WebCore/Modules/fetch/FetchBody.cpp:259 >> + ASSERT(data); > > It seems like this whole function should be replaced by switchOn. Right now it is not obvious that this assertion is correct. Could we leave this one as a RefPtr for now? OK, changed.
Yusuke Suzuki
Comment 21 2018-12-20 00:05:09 PST
View in context: https://bugs.webkit.org/attachment.cgi?id=357665&action=review Thanks! >> Source/JavaScriptCore/API/JSTypedArray.cpp:188 >> + Ref<ArrayBuffer> buffer = ArrayBuffer::createFromBytes(bytes, length, [=](void* p) { > > auto buffer? Fixed. >> Source/JavaScriptCore/runtime/JSArrayBufferPrototype.cpp:59 >> + Ref<ArrayBuffer> newBuffer = thisObject->impl()->slice(begin, end); > > auto newBuffer? Fixed. >> Source/WebCore/Modules/fetch/FetchBody.cpp:259 >> + ASSERT(data); > > It seems like this whole function should be replaced by switchOn. Right now it is not obvious that this assertion is correct. Could we leave this one as a RefPtr for now? OK, changed.
Yusuke Suzuki
Comment 22 2018-12-20 01:19:32 PST
Created attachment 357802 [details] Patch
Yusuke Suzuki
Comment 23 2018-12-20 08:21:02 PST
Created attachment 357812 [details] Patch
Yusuke Suzuki
Comment 24 2018-12-21 22:37:47 PST
Committed r239535: <https://trac.webkit.org/changeset/239535>
Radar WebKit Bug Importer
Comment 25 2018-12-21 22:38:27 PST
<rdar://problem/46916279>
Note You need to log in before you can comment on or make changes to this bug.