Bug 192808 – Use Ref<> as much as possible

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

Use Ref<> as much as possible

[Yusuke Suzuki]

Created attachment 357549 [details]
Patch

[Yusuke Suzuki]

Created attachment 357552 [details]
Patch

[Yusuke Suzuki]

Created attachment 357553 [details]
Patch

[Yusuke Suzuki]

Created attachment 357554 [details]
Patch

[Yusuke Suzuki]

Created attachment 357555 [details]
Patch

[Yusuke Suzuki]

Created attachment 357556 [details]
Patch

[Yusuke Suzuki]

Created attachment 357557 [details]
Patch

[Yusuke Suzuki]

Created attachment 357560 [details]
Patch

[Yusuke Suzuki]

Created attachment 357561 [details]
Patch

[Yusuke Suzuki]

Created attachment 357562 [details]
Patch

[Yusuke Suzuki]

Created attachment 357563 [details]
Patch

[Yusuke Suzuki]

Created attachment 357564 [details]
Patch

[Yusuke Suzuki]

Created attachment 357569 [details]
Patch

[Yusuke Suzuki]

Created attachment 357584 [details]
Patch

[Saam Barati]

Comment on attachment 357584 [details]
Patch

Seems like you have some build errors

[Yusuke Suzuki]

Created attachment 357665 [details]
Patch

[Yusuke Suzuki]

OK, the patch is ready now :)

[Alex Christensen]

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]

Created attachment 357796 [details]
Patch

[Yusuke Suzuki]

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]

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]

Created attachment 357802 [details]
Patch

[Yusuke Suzuki]

Created attachment 357812 [details]
Patch

[Yusuke Suzuki]

Committed r239535: <https://trac.webkit.org/changeset/239535>

[Radar WebKit Bug Importer]

<rdar://problem/46916279>