Bug 194829 – [WKTR] Avoid starting new NetworkProcesses unnecessarily when running the layout tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chris Dumez]

Avoid starting new NetworkProcesses unnecessarily when running the layout tests.

[Chris Dumez]

<rdar://problem/47889906>

[Chris Dumez]

Created attachment 362427 [details]
Patch

[Jonathan Bedard]

High-level: This is a good idea, probably has efficiency wins beyond just de-duplicating processes.

That being said, I'm a bit concerned that this is how we solved the problem. If the WKContext leak was in the test harness, then fair enough, but I get the sense that the WKContext leak is a general WebKit problem. Creating a new WKContext and deleting the old one is a supported workflow, right?

[Chris Dumez]

(In reply to Jonathan Bedard from comment #3)
> High-level: This is a good idea, probably has efficiency wins beyond just
> de-duplicating processes.
> 
> That being said, I'm a bit concerned that this is how we solved the problem.
> If the WKContext leak was in the test harness, then fair enough, but I get
> the sense that the WKContext leak is a general WebKit problem. Creating a
> new WKContext and deleting the old one is a supported workflow, right?

Only 1 WKContext is leaking (the first one). This patch does not fix that and I will investigate it too. This patch does not prevent us from seeing or fixing the leak of the first WKContext.

[Alexey Proskuryakov]

Comment on attachment 362427 [details]
Patch

r=me pending EWS

[EWS Watchlist]

Comment on attachment 362427 [details]
Patch

Attachment 362427 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11209559

New failing tests:
fast/dom/Geolocation/floorLevel.html

[EWS Watchlist]

Created attachment 362453 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6

[Chris Dumez]

Created attachment 362499 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 362499 [details]
Patch

Clearing flags on attachment: 362499

Committed r241821: <https://trac.webkit.org/changeset/241821>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48242710>

[Truitt Savell]

Interesting failure caused by https://trac.webkit.org/changeset/241821/webkit

fast/mediastream/MediaStream-video-element.html has become a constant failure on Mac WK2. the failure only reproduces when running this test with the test that runs right before it in the test runner. 

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fmediastream%2FMediaStream-video-element.html

repro:

run-webkit-tests --root testbuild-241821 fast/mediastream/MediaStream-video-element-video-tracks-disabled.html fast/mediastream/MediaStream-video-element.html --child-processes 1

the test fails on r241821 100% but passes on 241820.

[Truitt Savell]

fast/mediastream/MediaStream-video-element.html Diff:

--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/mediastream/MediaStream-video-element-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/fast/mediastream/MediaStream-video-element-actual.txt
@@ -47,7 +47,7 @@
 video.audioTracks[0] properties:
   track.id = <UUID>
   track.kind = main
-  track.label = Mock audio device 1
+  track.label = 
   track.language = 
   track.enabled = true
   track.sourceBuffer = null

[Jonathan Bedard]

(In reply to Truitt Savell from comment #13)
> fast/mediastream/MediaStream-video-element.html Diff:
> 
> ---
> /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/
> fast/mediastream/MediaStream-video-element-expected.txt
> +++
> /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/
> fast/mediastream/MediaStream-video-element-actual.txt
> @@ -47,7 +47,7 @@
>  video.audioTracks[0] properties:
>    track.id = <UUID>
>    track.kind = main
> -  track.label = Mock audio device 1
> +  track.label = 
>    track.language = 
>    track.enabled = true
>    track.sourceBuffer = null

I am of the opinion that this test failure means that this patch exposed another underlying WebKit bug, but did not cause this bug.

[Chris Dumez]

(In reply to Jonathan Bedard from comment #14)
> (In reply to Truitt Savell from comment #13)
> > fast/mediastream/MediaStream-video-element.html Diff:
> > 
> > ---
> > /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/
> > fast/mediastream/MediaStream-video-element-expected.txt
> > +++
> > /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/
> > fast/mediastream/MediaStream-video-element-actual.txt
> > @@ -47,7 +47,7 @@
> >  video.audioTracks[0] properties:
> >    track.id = <UUID>
> >    track.kind = main
> > -  track.label = Mock audio device 1
> > +  track.label = 
> >    track.language = 
> >    track.enabled = true
> >    track.sourceBuffer = null
> 
> I am of the opinion that this test failure means that this patch exposed
> another underlying WebKit bug, but did not cause this bug.

Agreed.


fast/mediastream/MediaStream-video-element-video-tracks-disabled.html usesenableModernMediaControls=false so it would have previously caused us to recreate a WebView and a WKContext. After my change, we now reuse the context and merely reconstruct a new WebView. This is somehow causing this flakiness.

[Alexey Proskuryakov]

Is there a new bug tracking the failure?

[Chris Dumez]

(In reply to Alexey Proskuryakov from comment #16)
> Is there a new bug tracking the failure?

https://bugs.webkit.org/show_bug.cgi?id=194916