Summary: | [WKTR] Avoid starting new NetworkProcesses unnecessarily when running the layout tests | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||
Component: | Tools / Tests | Assignee: | Chris Dumez <cdumez> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, ap, beidson, commit-queue, ews-watchlist, ggaren, graouts, jbedard, lforschler, tsavell, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=194916 | ||||||||||
Attachments: |
|
Description
Chris Dumez
2019-02-19 14:03:01 PST
Created attachment 362427 [details]
Patch
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? (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. Comment on attachment 362427 [details]
Patch
r=me pending EWS
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 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
Created attachment 362499 [details]
Patch
Comment on attachment 362499 [details] Patch Clearing flags on attachment: 362499 Committed r241821: <https://trac.webkit.org/changeset/241821> All reviewed patches have been landed. Closing bug. 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. 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 (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. (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. Is there a new bug tracking the failure? (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 |