Bug 149449

Summary: Reduce almost uses of PassRefPtr in Webcore/testing
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: New BugsAssignee: Gyuyoung Kim <gyuyoung.kim>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Gyuyoung Kim 2015-09-21 23:16:10 PDT
I fail to remove PassRefPtr in nternals::serializeObject() and Internals::deserializeObject(). I want to remove it in next patch again.
Comment 1 Gyuyoung Kim 2015-09-21 23:17:21 PDT
Created attachment 261728 [details]
Patch
Comment 2 Gyuyoung Kim 2015-09-22 08:22:48 PDT
Created attachment 261743 [details]
Patch
Comment 3 Darin Adler 2015-09-22 09:23:17 PDT
Comment on attachment 261743 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261743&action=review

r=me but please don’t land the incorrect change to JSTestOverrideBuiltins.cpp

> Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.cpp:220
> -    JSValue result = toJS(state, castedThis->globalObject(), WTF::getPtr(impl.namedItem(name)));
> +    JSValue result = toJS(state, castedThis->globalObject(), WTF::getPtr(impl.namedItem(WTF::move(name))));

This is an expected test result for the bindings generator test, not a source file.

To fix this, we need to change the code that generates this file, not change the file itself.

If you land this change as is, it will just cause the test to start failing because this file doesn’t match what the generator generates. The code for this is deep inside the GenerateImplementation function in the file CodeGeneratorJS.pm.

> Source/WebCore/testing/MallocStatistics.h:36
> +    static Ref<MallocStatistics> create() { return adoptRef(*new MallocStatistics()); }

No need for the () after MallocStatistics.
Comment 4 Gyuyoung Kim 2015-09-23 09:01:23 PDT
Created attachment 261824 [details]
Patch
Comment 5 Gyuyoung Kim 2015-09-23 09:06:09 PDT
Comment on attachment 261743 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=261743&action=review

>> Source/WebCore/bindings/scripts/test/JS/JSTestOverrideBuiltins.cpp:220
>> +    JSValue result = toJS(state, castedThis->globalObject(), WTF::getPtr(impl.namedItem(WTF::move(name))));
> 
> This is an expected test result for the bindings generator test, not a source file.
> 
> To fix this, we need to change the code that generates this file, not change the file itself.
> 
> If you land this change as is, it will just cause the test to start failing because this file doesn’t match what the generator generates. The code for this is deep inside the GenerateImplementation function in the file CodeGeneratorJS.pm.

I see. I was wrong. I remove WTF::move() in this line. Thanks.

>> Source/WebCore/testing/MallocStatistics.h:36
>> +    static Ref<MallocStatistics> create() { return adoptRef(*new MallocStatistics()); }
> 
> No need for the () after MallocStatistics.

done.
Comment 6 WebKit Commit Bot 2015-09-24 07:57:13 PDT
Comment on attachment 261824 [details]
Patch

Clearing flags on attachment: 261824

Committed r190202: <http://trac.webkit.org/changeset/190202>
Comment 7 WebKit Commit Bot 2015-09-24 07:57:18 PDT
All reviewed patches have been landed.  Closing bug.