Bug 206672 – REGRESSION: [iOS release] http/tests/security/window-named-proto.html is a flaky timing out

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Jason Lawrence]

http/tests/security/window-named-proto.html

Description:
This test is timing out. It looks like this regressed around 252941. I have not tried to reproduce this.

History:
https://results.webkit.org/?limit=50000&suite=layout-tests&test=http%2Ftests%2Fsecurity%2Fwindow-named-proto.html

Diff:
--- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/http/tests/security/window-named-proto-expected.txt
+++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/http/tests/security/window-named-proto-actual.txt
@@ -1,2 +1,3 @@
-CONSOLE MESSAGE: line 2: TypeError: null is not an object (evaluating 'document.body.innerHTML')
+#PID UNRESPONSIVE - WebKitTestRunnerApp (pid 29959)
+FAIL: Timed out waiting for notifyDone to be called

[Radar WebKit Bug Importer]

<rdar://problem/58838583>

[Truitt Savell]

This is a highly intermittent timeout on iOS iOS Release

[Alexey Proskuryakov]

When I increase history limit, I see even older timeouts, starting with r251319. It is pretty rare, so history doesn't point at the culprit, but this is sort of consistent with HTML event loop changes.

[Jiewen Tan]

Created attachment 389286 [details]
Patch

[Jiewen Tan]

Created attachment 389288 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 389288 [details]
Patch

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

> Source/WebCore/platform/network/DataURLDecoder.cpp:46
> -    static auto& queue = WorkQueue::create("org.webkit.DataURLDecoder").leakRef();
> +    static auto& queue = WorkQueue::create("org.webkit.DataURLDecoder", WorkQueue::Type::Serial, WorkQueue::QOS::UserInitiated).leakRef();

Hm... what's the default QOS? I thought all threads we start will be at least user initiated by default?

[Jiewen Tan]

(In reply to Ryosuke Niwa from comment #6)
> Comment on attachment 389288 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389288&action=review
> 
> > Source/WebCore/platform/network/DataURLDecoder.cpp:46
> > -    static auto& queue = WorkQueue::create("org.webkit.DataURLDecoder").leakRef();
> > +    static auto& queue = WorkQueue::create("org.webkit.DataURLDecoder", WorkQueue::Type::Serial, WorkQueue::QOS::UserInitiated).leakRef();
> 
> Hm... what's the default QOS? I thought all threads we start will be at
> least user initiated by default?

static Ref<WorkQueue> create(const char* name, Type = Type::Serial, QOS = QOS::Default);

[Chris Dumez]

Comment on attachment 389288 [details]
Patch

Makes sense to use user-initiated IMO since this is the priority on the WebContent process' main thread and since this thread getting pre-empted would slow down a load, which would be visible to the user.

[Ryosuke Niwa]

(In reply to Chris Dumez from comment #8)
> Comment on attachment 389288 [details]
> Patch
> 
> Makes sense to use user-initiated IMO since this is the priority on the
> WebContent process' main thread and since this thread getting pre-empted
> would slow down a load, which would be visible to the user.

FWIW, I'm pretty sure WebContent's main thread runs at user interactive QOS, which is slightly higher priority than user-initiated QOS.

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #9)
> (In reply to Chris Dumez from comment #8)
> > Comment on attachment 389288 [details]
> > Patch
> > 
> > Makes sense to use user-initiated IMO since this is the priority on the
> > WebContent process' main thread and since this thread getting pre-empted
> > would slow down a load, which would be visible to the user.
> 
> FWIW, I'm pretty sure WebContent's main thread runs at user interactive QOS,
> which is slightly higher priority than user-initiated QOS.

Not 100% sure about that, would need to be validated on device to be sure. Either way, I do think that using user-initiated for data url is better than Default. I think it would be riskier to use UserInteractive.

[Jiewen Tan]

Comment on attachment 389288 [details]
Patch

Chris, thanks for r+ this patch.

[WebKit Commit Bot]

Comment on attachment 389288 [details]
Patch

Clearing flags on attachment: 389288

Committed r255487: <https://trac.webkit.org/changeset/255487>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.