Bug 224356

Summary:

ServiceWorker should save module scripts

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	youennf: review+, ews-feeder: commit-queue-

Yusuke Suzuki

Reported 2021-04-08 19:36:37 PDT

ServiceWorker saves module scripts

Attachments
Patch (13.72 KB, patch) 2021-04-08 19:37 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (16.15 KB, patch) 2021-04-08 19:42 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (17.72 KB, patch) 2021-04-08 19:52 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (17.73 KB, patch) 2021-04-08 20:08 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (17.82 KB, patch) 2021-04-08 20:30 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (18.43 KB, patch) 2021-04-08 21:00 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (18.45 KB, patch) 2021-04-08 21:19 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (14.85 KB, patch) 2021-04-08 21:46 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (14.85 KB, patch) 2021-04-08 22:11 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (16.01 KB, patch) 2021-04-08 23:03 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (16.17 KB, patch) 2021-04-08 23:05 PDT, Yusuke Suzuki	youennf: review+ ews-feeder: commit-queue-	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2021-04-08 19:37:57 PDT

Created attachment 425573 [details] Patch

Yusuke Suzuki

Comment 2 2021-04-08 19:38:00 PDT

<rdar://problem/75634897>

Yusuke Suzuki

Comment 3 2021-04-08 19:42:19 PDT

Created attachment 425574 [details] Patch

Yusuke Suzuki

Comment 4 2021-04-08 19:52:38 PDT

Created attachment 425576 [details] Patch

Yusuke Suzuki

Comment 5 2021-04-08 20:08:48 PDT

Created attachment 425578 [details] Patch

Yusuke Suzuki

Comment 6 2021-04-08 20:30:10 PDT

Created attachment 425579 [details] Patch

Yusuke Suzuki

Comment 7 2021-04-08 21:00:53 PDT

Created attachment 425582 [details] Patch

Yusuke Suzuki

Comment 8 2021-04-08 21:19:56 PDT

Created attachment 425583 [details] Patch

Yusuke Suzuki

Comment 9 2021-04-08 21:46:47 PDT

Created attachment 425585 [details] Patch

Yusuke Suzuki

Comment 10 2021-04-08 22:11:50 PDT

Created attachment 425586 [details] Patch

Yusuke Suzuki

Comment 11 2021-04-08 22:31:06 PDT

We found that existing test mechanism "testRunner.terminateNetworkProcess" does not work well with guard-malloc configuration. https://bugs.webkit.org/show_bug.cgi?id=224358

Yusuke Suzuki

Comment 12 2021-04-08 23:03:08 PDT

Created attachment 425588 [details] Patch

Yusuke Suzuki

Comment 13 2021-04-08 23:05:59 PDT

Created attachment 425589 [details] Patch

Yusuke Suzuki

Comment 14 2021-04-08 23:34:16 PDT

Comment on attachment 425589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425589&action=review > LayoutTests/http/wpt/service-workers/network-process-termination.html:21 > +<html> > +<head> > +<script src="resources/routines.js"></script> > +</head> > +<body> > +<script> > +if (window.testRunner) { > + testRunner.dumpAsText(); > + testRunner.waitUntilDone(); > +} > +async function doTest() { > + testRunner.terminateNetworkProcess(); > + await waitFor(100); > + document.body.innerHTML = "PASS"; > + if (window.testRunner) > + testRunner.notifyDone(); > +} > +doTest(); > +</script> > +</body> > +</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

Yusuke Suzuki

Comment 15 2021-04-08 23:50:41 PDT

Comment on attachment 425589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425589&action=review >> 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.

youenn fablet

Comment 16 2021-04-09 01:35:09 PDT

Comment on attachment 425589 [details] 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?

Yusuke Suzuki

Comment 17 2021-04-09 01:37:12 PDT

Comment on attachment 425589 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=425589&action=review Thanks! >> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:191 >> + // This can invoke callback immediately. > > This comment is not very clear to me. OK, removed. >> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:411 >> + m_requestURLToResponseURLMap.add(sourceURL.string(), responseURL); > > WTFMove(responseURL) Looks good! >> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:459 >> + m_requestURLToResponseURLMap.add(sourceURL.string(), responseURL); > > WTFMove We still use responseURL (L462). >>>> 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? Sounds good. I'll upload this test in a filed bugzilla.

Yusuke Suzuki

Comment 18 2021-04-09 01:54:47 PDT

Committed r275751 (236329@main): <https://commits.webkit.org/236329@main>

Yusuke Suzuki

Comment 19 2021-04-09 20:05:37 PDT

*** Bug 223135 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.