Bug 188880 – Assert in NetworkBlobRegistry::unregisterBlobURL after network process had terminated

• Component: WebKit2
• Status: RESOLVED FIXED

[Ryosuke Niwa]

When a network process terminates, it obviously loses track of the registered blobs.
When WebContent process then tries to unregister a blob, we hit a debug assertion in NetworkBlobRegistry::unregisterBlobURL.

[Ryosuke Niwa]

Created attachment 347902 [details]
Removes the assertion

[EWS Watchlist]

Attachment 347902 [details] did not pass style-queue:


ERROR: LayoutTests/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Ryosuke Niwa]

Created attachment 347903 [details]
Removes the assertion

[Saam Barati]

Comment on attachment 347903 [details]
Removes the assertion

r=me

[EWS Watchlist]

Comment on attachment 347903 [details]
Removes the assertion

Attachment 347903 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8954594

New failing tests:
fast/files/blob-network-process-crash.html

[EWS Watchlist]

Created attachment 347911 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Ryosuke Niwa]

Oh oops, clearly we have to skip this test in Wk1.

[Ryosuke Niwa]

Created attachment 347913 [details]
Patch for landing

[Ryosuke Niwa]

Comment on attachment 347913 [details]
Patch for landing

Waiting for EWS

[EWS Watchlist]

Attachment 347913 [details] did not pass style-queue:


ERROR: LayoutTests/platform/wk2/TestExpectations:744:  expecting "[", "#", or end of line instead of "Pass"  [test/expectations] [5]
ERROR: LayoutTests/platform/wk2/TestExpectations:744:  Path does not exist.  [test/expectations] [5]
ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/wk2/TestExpectations:744:  expecting "[", "#", or end of line instead of "Pass"  [test/expectations] [5]
ERROR: /Volumes/Data/StyleQueue/WebKit/LayoutTests/platform/wk2/TestExpectations:744:  Path does not exist.  [test/expectations] [5]
Total errors found: 4 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alex Christensen]

Comment on attachment 347913 [details]
Patch for landing

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

> LayoutTests/TestExpectations:393
> +# testRunner.terminateNetworkProcess() only works in WebKit2
> +fast/files/blob-network-process-crash.html [ Skip ]

We could've just put if (testRunner.terminateNetworkProcess) instead of this.

[Ryosuke Niwa]

(In reply to Alex Christensen from comment #11)
> Comment on attachment 347913 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347913&action=review
> 
> > LayoutTests/TestExpectations:393
> > +# testRunner.terminateNetworkProcess() only works in WebKit2
> > +fast/files/blob-network-process-crash.html [ Skip ]
> 
> We could've just put if (testRunner.terminateNetworkProcess) instead of this.

But then the test doesn't do anything useful.

[Ryosuke Niwa]

(In reply to Ryosuke Niwa from comment #12)
> (In reply to Alex Christensen from comment #11)
> > Comment on attachment 347913 [details]
> > Patch for landing
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=347913&action=review
> > 
> > > LayoutTests/TestExpectations:393
> > > +# testRunner.terminateNetworkProcess() only works in WebKit2
> > > +fast/files/blob-network-process-crash.html [ Skip ]
> > 
> > We could've just put if (testRunner.terminateNetworkProcess) instead of this.
> 
> But then the test doesn't do anything useful.

That is, since this test isn't useful in DRT, there is no point in running it there.

[Ryosuke Niwa]

Committed r235243: <https://trac.webkit.org/changeset/235243>

[Radar WebKit Bug Importer]

<rdar://problem/43656323>

[Ryan Haddad]

The layout test added with this change is timing out on the bots:

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r235245%20(6350)/results.html

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Ffiles%2Fblob-network-process-crash.html

--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/files/blob-network-process-crash-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/files/blob-network-process-crash-actual.txt
@@ -1,2 +1,6 @@
-This tests unregistering a Blob object after the network process to which it was registered crashed.
-WebKit should not hit a debug assertion in the network process.
+CONSOLE MESSAGE: Unhandled Promise Rejection: TypeError: WebKit encountered an internal error
+#PID UNRESPONSIVE - com.apple.WebKit.WebContent.Development (pid 38955)
+FAIL: Timed out waiting for notifyDone to be called
+
+#EOF
+#EOF

[Ryosuke Niwa]

Investigating...

[Ryosuke Niwa]

This is super weird. Timer sometimes never fires in this test.
Tracking this bug in https://bugs.webkit.org/show_bug.cgi?id=188911.

Temporarily worked around the issue in <https://trac.webkit.org/changeset/235264 to unflake the test.