REGRESSION: http/tests/adClickAttribution/second-attribution-converted-with-higher-priority.html and http/tests/adClickAttribution/second-attribution-converted-with-lower-priority.html are flaky timeouts

[Ryan Haddad]

http/tests/adClickAttribution/second-attribution-converted-with-higher-priority.html and http/tests/adClickAttribution/second-attribution-converted-with-lower-priority.html have recently become flaky timeouts on macOS WK2 queues.

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2FadClickAttribution%2Fsecond-attribution-converted-with-higher-priority.html%20http%2Ftests%2FadClickAttribution%2Fsecond-attribution-converted-with-lower-priority.html

The earliest blamed revision range is https://trac.webkit.org/log/webkit/?verbose=on&rev=249350&stop_rev=249349

[Radar WebKit Bug Importer]

<rdar://problem/54998427>

[Russell Epstein]

I was able to reproduce this timeout starting with r249350 using the following command:

run-webkit-tests --iter 1000 -f --no-sample-on-timeout --no-retry http/tests/adClickAttribution/second-attribution-converted-with-higher-priority.html

[Chris Dumez]

It is weird, when it times out, I actually see the test running to completion and calling testRunner.notifyDone(). This seems like a potential issue with WebKitTestRunner where it fails to stop even though notifyDone() was called.

[Chris Dumez]

(In reply to Chris Dumez from comment #3)
> It is weird, when it times out, I actually see the test running to
> completion and calling testRunner.notifyDone(). This seems like a potential
> issue with WebKitTestRunner where it fails to stop even though notifyDone()
> was called.

It seems the InjectedBundle in the WebContent process is sending the "Done" IPC to the UIProcess but this IPC is not getting received in the UIProcess.

[Chris Dumez]

(In reply to Chris Dumez from comment #4)
> (In reply to Chris Dumez from comment #3)
> > It is weird, when it times out, I actually see the test running to
> > completion and calling testRunner.notifyDone(). This seems like a potential
> > issue with WebKitTestRunner where it fails to stop even though notifyDone()
> > was called.
> 
> It seems the InjectedBundle in the WebContent process is sending the "Done"
> IPC to the UIProcess but this IPC is not getting received in the UIProcess.

I have confirmed that the WebProcessProxy is dropping this IPC message on the floor (likely because there is not WebPageProxy listening anymore for this identifier). I am investigating to find out why.

[Chris Dumez]

(In reply to Chris Dumez from comment #5)
> (In reply to Chris Dumez from comment #4)
> > (In reply to Chris Dumez from comment #3)
> > > It is weird, when it times out, I actually see the test running to
> > > completion and calling testRunner.notifyDone(). This seems like a potential
> > > issue with WebKitTestRunner where it fails to stop even though notifyDone()
> > > was called.
> > 
> > It seems the InjectedBundle in the WebContent process is sending the "Done"
> > IPC to the UIProcess but this IPC is not getting received in the UIProcess.
> 
> I have confirmed that the WebProcessProxy is dropping this IPC message on
> the floor (likely because there is not WebPageProxy listening anymore for
> this identifier). I am investigating to find out why.

It looks like the WebKitTestRunner's injected bundle gets confused when we use a process for a new load that already had a WKTR injected bundle. It still tries to talk to the previous page in that process (which is now suspended).

[Chris Dumez]

(In reply to Chris Dumez from comment #6)
> (In reply to Chris Dumez from comment #5)
> > (In reply to Chris Dumez from comment #4)
> > > (In reply to Chris Dumez from comment #3)
> > > > It is weird, when it times out, I actually see the test running to
> > > > completion and calling testRunner.notifyDone(). This seems like a potential
> > > > issue with WebKitTestRunner where it fails to stop even though notifyDone()
> > > > was called.
> > > 
> > > It seems the InjectedBundle in the WebContent process is sending the "Done"
> > > IPC to the UIProcess but this IPC is not getting received in the UIProcess.
> > 
> > I have confirmed that the WebProcessProxy is dropping this IPC message on
> > the floor (likely because there is not WebPageProxy listening anymore for
> > this identifier). I am investigating to find out why.
> 
> It looks like the WebKitTestRunner's injected bundle gets confused when we
> use a process for a new load that already had a WKTR injected bundle. It
> still tries to talk to the previous page in that process (which is now
> suspended).

I think the issue lies here:
InjectedBundlePage* InjectedBundle::page() const
{
    // It might be better to have the UI process send over a reference to the main
    // page instead of just assuming it's the first one.
    return m_pages[0].get();
}

We always assume the *first* page that was created in the process is the one we want to talk to.

[Chris Dumez]

Created attachment 378106 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 378106 [details]
Patch

Clearing flags on attachment: 378106

Committed r249557: <https://trac.webkit.org/changeset/249557>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.