[webkit-reviews] review denied: [Bug 194047] Integrate JSC bytecode cache with WebKit : [Attachment 360620] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 4 09:42:11 PST 2019

Antti Koivisto <koivisto at iki.fi> has denied Tadeu Zagallo
<tzagallo at apple.com>'s request for review:
Bug 194047: Integrate JSC bytecode cache with WebKit

Attachment 360620: Patch


--- Comment #5 from Antti Koivisto <koivisto at iki.fi> ---
Comment on attachment 360620
  --> https://bugs.webkit.org/attachment.cgi?id=360620

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

Looks generally good but lets get it apply before r+ing.

>>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:850
>>> +	     Vector<IPC::DerivedData>* derivedData = new
>> No need to call new here.  Just WTFMove an instance into the lambda capture
> That would move it multiple times.

Please use smart pointers for memory management. Calling new (outside create()
type functions that return smart pointers) is discouraged. 

auto derivedData = std::make_unique<Vector<IPC::DerivedData>>();

>>> Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp:857
>>> +			 return;
>> Why?
> Not sure actually, this was part of the code that you had deleted and I added
back. I assume that if this is the only ref to the loader, then we don't care
about the response anymore?

This lambda is then the only ref keeping the loader alive which implies there
are no clients. A more stylish solution would use WeakPtr.

More information about the webkit-reviews mailing list