Bug 192808 - Use Ref<> as much as possible
Summary: Use Ref<> as much as possible
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-12-18 00:56 PST by Yusuke Suzuki
Modified: 2018-12-21 22:38 PST (History)
8 users (show)

See Also:


Attachments
Patch (377.18 KB, patch)
2018-12-18 00:57 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (377.75 KB, patch)
2018-12-18 01:14 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (380.40 KB, patch)
2018-12-18 01:31 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (382.12 KB, patch)
2018-12-18 01:37 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (382.39 KB, patch)
2018-12-18 02:02 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (389.05 KB, patch)
2018-12-18 02:40 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (389.14 KB, patch)
2018-12-18 03:05 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (389.14 KB, patch)
2018-12-18 03:37 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (396.74 KB, patch)
2018-12-18 04:00 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (397.14 KB, patch)
2018-12-18 05:40 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (398.20 KB, patch)
2018-12-18 06:07 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (398.23 KB, patch)
2018-12-18 06:28 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (398.33 KB, patch)
2018-12-18 07:46 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (398.33 KB, patch)
2018-12-18 11:06 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (398.54 KB, patch)
2018-12-19 00:22 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (397.24 KB, patch)
2018-12-20 00:02 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (397.29 KB, patch)
2018-12-20 01:19 PST, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (399.70 KB, patch)
2018-12-20 08:21 PST, Yusuke Suzuki
achristensen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-12-18 00:56:36 PST
Use Ref<> as much as possible
Comment 1 Yusuke Suzuki 2018-12-18 00:57:36 PST
Created attachment 357549 [details]
Patch
Comment 2 Yusuke Suzuki 2018-12-18 01:14:18 PST
Created attachment 357552 [details]
Patch
Comment 3 Yusuke Suzuki 2018-12-18 01:31:38 PST
Created attachment 357553 [details]
Patch
Comment 4 Yusuke Suzuki 2018-12-18 01:37:36 PST
Created attachment 357554 [details]
Patch
Comment 5 Yusuke Suzuki 2018-12-18 02:02:54 PST
Created attachment 357555 [details]
Patch
Comment 6 Yusuke Suzuki 2018-12-18 02:40:46 PST
Created attachment 357556 [details]
Patch
Comment 7 Yusuke Suzuki 2018-12-18 03:05:12 PST
Created attachment 357557 [details]
Patch
Comment 8 Yusuke Suzuki 2018-12-18 03:37:17 PST
Created attachment 357560 [details]
Patch
Comment 9 Yusuke Suzuki 2018-12-18 04:00:53 PST
Created attachment 357561 [details]
Patch
Comment 10 Yusuke Suzuki 2018-12-18 05:40:15 PST
Created attachment 357562 [details]
Patch
Comment 11 Yusuke Suzuki 2018-12-18 06:07:59 PST
Created attachment 357563 [details]
Patch
Comment 12 Yusuke Suzuki 2018-12-18 06:28:58 PST
Created attachment 357564 [details]
Patch
Comment 13 Yusuke Suzuki 2018-12-18 07:46:38 PST
Created attachment 357569 [details]
Patch
Comment 14 Yusuke Suzuki 2018-12-18 11:06:27 PST
Created attachment 357584 [details]
Patch
Comment 15 Saam Barati 2018-12-18 23:28:29 PST
Comment on attachment 357584 [details]
Patch

Seems like you have some build errors
Comment 16 Yusuke Suzuki 2018-12-19 00:22:47 PST
Created attachment 357665 [details]
Patch
Comment 17 Yusuke Suzuki 2018-12-19 02:01:05 PST
OK, the patch is ready now :)
Comment 18 Alex Christensen 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?
Comment 19 Yusuke Suzuki 2018-12-20 00:02:54 PST
Created attachment 357796 [details]
Patch
Comment 20 Yusuke Suzuki 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.
Comment 21 Yusuke Suzuki 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.
Comment 22 Yusuke Suzuki 2018-12-20 01:19:32 PST
Created attachment 357802 [details]
Patch
Comment 23 Yusuke Suzuki 2018-12-20 08:21:02 PST
Created attachment 357812 [details]
Patch
Comment 24 Yusuke Suzuki 2018-12-21 22:37:47 PST
Committed r239535: <https://trac.webkit.org/changeset/239535>
Comment 25 Radar WebKit Bug Importer 2018-12-21 22:38:27 PST
<rdar://problem/46916279>