[webkit-reviews] review granted: [Bug 224356] ServiceWorker should save module scripts : [Attachment 425589] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 224356: ServiceWorker should save module scripts
https://bugs.webkit.org/show_bug.cgi?id=224356

Attachment 425589: Patch

https://bugs.webkit.org/attachment.cgi?id=425589&action=review




--- Comment #16 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 425589
  --> https://bugs.webkit.org/attachment.cgi?id=425589
Patch

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

> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:191
> +	   // This can invoke callback immediately.

This comment is not very clear to me.

> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:411
> +	   m_requestURLToResponseURLMap.add(sourceURL.string(), responseURL);

WTFMove(responseURL)

> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:459
> +	   m_requestURLToResponseURLMap.add(sourceURL.string(), responseURL);

WTFMove

>>> LayoutTests/http/wpt/service-workers/network-process-termination.html:21
>>> +</html>
>> 
>> I added this test to show that existing testRunner.terminateNetworkProcess()
is not working well with mac-wk2-stress's guard-malloc runs. (possibly, killing
the process does not work well for guard-malloc runs?).
>> But this is not an issue of this patch itself since it is just using the
existing test mechanism (we have many tests using this).
>> This issue is filed separately in
https://bugs.webkit.org/show_bug.cgi?id=224358
> 
> Note that this worked on local working environment with --guard-malloc. And
since guard-malloc bot does not report failures about terminateNetworkProcess,
this failure is specific to new EWS mac-wk2-stress's configuration.

Maybe this test should be in its own patch?