Bug 195758 – Delay WebProcess launch until a load is triggered in a Web view

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Delay WebProcess launch until a load is triggered in a WebView instead of launching it as soon as the Web view is created.
This would allow leveraging the WebProcessCache given that we would know then which registrable domain is going to be loaded in the process.

[Chris Dumez]

<rdar://problem/48126013>

[Chris Dumez]

Created attachment 364710 [details]
WIP Patch

[Chris Dumez]

Created attachment 364713 [details]
Patch

[Chris Dumez]

Created attachment 364717 [details]
Patch

[Chris Dumez]

Created attachment 364718 [details]
Patch

[EWS Watchlist]

Comment on attachment 364718 [details]
Patch

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

New failing tests:
fast/text/international/system-language/declarative-language.html
imported/w3c/web-platform-tests/html/browsers/the-window-object/security-window/window-security.sub.html
fast/dom/Window/showModalDialog-mandatory-parameter.html
fast/dom/Window/window-function-name-getter-precedence.html
fast/dom/Window/window-function-frame-getter-precedence.html
fast/dom/Window/delete-operations.html
http/tests/security/cross-frame-access-call.html
js/dom/function-length.html
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html

[EWS Watchlist]

Created attachment 364736 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 364718 [details]
Patch

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

New failing tests:
fast/visual-viewport/ios/min-scale-greater-than-one.html
fast/css/preserve-user-specified-zoom-level-on-reload.html
platform/ios/ios/fast/coordinates/inner-window-sizes-quirks.html
fast/dom/Window/window-function-name-getter-precedence.html
platform/ipad/fast/viewport/empty-meta.html
fast/viewport/ios/use-minimum-device-width-for-page-without-viewport-meta.html
fast/dom/Window/showModalDialog-mandatory-parameter.html
fast/viewport/ios/viewport-minimum-and-initial-scale.html
fast/dom/Window/delete-operations.html
platform/ipad/fast/viewport/viewport-unchanged-by-minimum-effective-width-if-not-ignore-meta-viewport.html
fast/viewport/ios/constant-width-viewport-after-changing-view-scale.html
media/modern-media-controls/media-controller/ios/media-controller-scale-factor-audio.html
fast/scrolling/adjust-scroll-offset-on-zoom.html
fast/text/international/system-language/declarative-language.html
platform/ios/ios/fast/coordinates/inner-window-sizes.html
fast/viewport/ios/width-is-device-width-overflowing-body-overflow-hidden-tall.html
platform/ipad/fast/viewport/viewport-overriden-by-minimum-effective-width-if-ignore-meta-viewport.html
editing/input/ios/rtl-keyboard-input-on-focus.html
fast/viewport/ios/initial-scale-after-changing-view-scale.html
imported/w3c/web-platform-tests/html/browsers/the-window-object/security-window/window-security.sub.html
fast/dom/Window/window-function-frame-getter-precedence.html
js/dom/function-length.html
http/tests/security/cross-frame-access-call.html
media/modern-media-controls/media-controller/ios/media-controller-scale-factor.html

[EWS Watchlist]

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

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

[Chris Dumez]

Created attachment 364811 [details]
Patch

[Chris Dumez]

Created attachment 364813 [details]
Patch

[EWS Watchlist]

Comment on attachment 364813 [details]
Patch

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

New failing tests:
fast/text/international/system-language/declarative-language.html

[EWS Watchlist]

Created attachment 364820 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Chris Dumez]

Created attachment 364838 [details]
Patch

[Chris Dumez]

Created attachment 364840 [details]
Patch

[EWS Watchlist]

Comment on attachment 364840 [details]
Patch

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

New failing tests:
http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html

[EWS Watchlist]

Created attachment 364858 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Chris Dumez]

(In reply to Build Bot from comment #16)
> Comment on attachment 364840 [details]
> Patch
> 
> Attachment 364840 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: https://webkit-queues.webkit.org/results/11522959
> 
> New failing tests:
> http/wpt/mediarecorder/MediaRecorder-AV-audio-video-dataavailable.html

Flaky test, crashes in media code.

[EWS Watchlist]

Comment on attachment 364840 [details]
Patch

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

New failing tests:
editing/input/ios/rtl-keyboard-input-on-focus.html
fast/viewport/ios/initial-scale-after-changing-view-scale.html

[EWS Watchlist]

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

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

[Chris Dumez]

Created attachment 364886 [details]
Patch

[Chris Dumez]

Created attachment 364892 [details]
Patch

[Chris Dumez]

Created attachment 364908 [details]
Patch

[Chris Dumez]

Created attachment 364909 [details]
Patch

[Chris Dumez]

Created attachment 364914 [details]
Patch

[EWS Watchlist]

Comment on attachment 364914 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html

[EWS Watchlist]

Created attachment 364918 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Chris Dumez]

Comment on attachment 364918 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

Flaky media recorder again.

[Chris Dumez]

Created attachment 364998 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebKit/Shared/API/Cocoa/RemoteObjectRegistry.mm:52:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 40 files


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

[Chris Dumez]

Created attachment 365000 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebKit/Shared/API/Cocoa/RemoteObjectRegistry.mm:52:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 40 files


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

[Geoffrey Garen]

Comment on attachment 365000 [details]
Patch

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

r=me

> Source/WebKit/UIProcess/WebPageProxy.cpp:3560
> +    if (!hasRunningProcess() && &process() == process().processPool().initialProcessForPages())

Maybe turn !hasRunningProces() into an ASSERT.

> Source/WebKit/UIProcess/WebProcessPool.h:167
> +    WebProcessProxy* initialProcessForPages() const { return m_initialProcessForPages; }

I think I would call this dummyProcessProxy(), or something similar. "Initial" is how its clients use it, but not really how it behaves or what it is.

Maybe also add a comment just to explain that this is a convenience object to avoid null checks, and it doesn't intend to carry any semantic meaning.

[Chris Dumez]

Created attachment 365038 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebKit/Shared/API/Cocoa/RemoteObjectRegistry.mm:52:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 40 files


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

[EWS Watchlist]

Comment on attachment 365038 [details]
Patch

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

New failing tests:
http/tests/security/cross-origin-worker-indexeddb.html

[EWS Watchlist]

Created attachment 365046 [details]
Archive of layout-test-results from ews113 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[Chris Dumez]

Comment on attachment 365046 [details]
Archive of layout-test-results from ews113 for mac-highsierra

Cannot impact wk1.

[WebKit Commit Bot]

Comment on attachment 365038 [details]
Patch

Clearing flags on attachment: 365038

Committed r243094: <https://trac.webkit.org/changeset/243094>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/243094/webkit

appears to have broken media/track/track-in-band-style.html

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-style.html

Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/track/track-in-band-style-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/track/track-in-band-style-actual.txt
@@ -14,6 +14,6 @@
 
 ** Test current cue colors
 EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255, 255)') OK
-EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
+EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'), OBSERVED 'rgba(0, 0, 0, 0)' FAIL
 END OF TEST
 

I reproduced this on 243094 and saw no failures when running it on 243092. 243093 is a versioning patch.

[Chris Dumez]

(In reply to Truitt Savell from comment #41)
> The changes in https://trac.webkit.org/changeset/243094/webkit
> 
> appears to have broken media/track/track-in-band-style.html
> 
> History:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-style.html
> 
> Diff:
> ---
> /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> track/track-in-band-style-expected.txt
> +++
> /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> track/track-in-band-style-actual.txt
> @@ -14,6 +14,6 @@
>  
>  ** Test current cue colors
>  EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255,
> 255)') OK
> -EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
> +EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'),
> OBSERVED 'rgba(0, 0, 0, 0)' FAIL
>  END OF TEST
>  
> 
> I reproduced this on 243094 and saw no failures when running it on 243092.
> 243093 is a versioning patch.

EWS was green and the tests were passing locally so this is odd. I am investigating...

[Chris Dumez]

(In reply to Chris Dumez from comment #42)
> (In reply to Truitt Savell from comment #41)
> > The changes in https://trac.webkit.org/changeset/243094/webkit
> > 
> > appears to have broken media/track/track-in-band-style.html
> > 
> > History:
> > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-style.html
> > 
> > Diff:
> > ---
> > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > track/track-in-band-style-expected.txt
> > +++
> > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > track/track-in-band-style-actual.txt
> > @@ -14,6 +14,6 @@
> >  
> >  ** Test current cue colors
> >  EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255,
> > 255)') OK
> > -EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
> > +EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'),
> > OBSERVED 'rgba(0, 0, 0, 0)' FAIL
> >  END OF TEST
> >  
> > 
> > I reproduced this on 243094 and saw no failures when running it on 243092.
> > 243093 is a versioning patch.
> 
> EWS was green and the tests were passing locally so this is odd. I am
> investigating...

It does not reproduce locally. Also note that 'rgb(0, 0, 0)') and 'rgba(0, 0, 0, 0)' are not terribly different. They are still the same color.

[Chris Dumez]

(In reply to Chris Dumez from comment #43)
> (In reply to Chris Dumez from comment #42)
> > (In reply to Truitt Savell from comment #41)
> > > The changes in https://trac.webkit.org/changeset/243094/webkit
> > > 
> > > appears to have broken media/track/track-in-band-style.html
> > > 
> > > History:
> > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > > html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-style.html
> > > 
> > > Diff:
> > > ---
> > > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > > track/track-in-band-style-expected.txt
> > > +++
> > > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > > track/track-in-band-style-actual.txt
> > > @@ -14,6 +14,6 @@
> > >  
> > >  ** Test current cue colors
> > >  EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255,
> > > 255)') OK
> > > -EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
> > > +EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'),
> > > OBSERVED 'rgba(0, 0, 0, 0)' FAIL
> > >  END OF TEST
> > >  
> > > 
> > > I reproduced this on 243094 and saw no failures when running it on 243092.
> > > 243093 is a versioning patch.
> > 
> > EWS was green and the tests were passing locally so this is odd. I am
> > investigating...
> 
> It does not reproduce locally. Also note that 'rgb(0, 0, 0)') and 'rgba(0,
> 0, 0, 0)' are not terribly different. They are still the same color.

This seems to rely on Color::cssText() so Color::isOpaque() returns false and we use rgba() instead of rgb() format.

[Chris Dumez]

(In reply to Chris Dumez from comment #44)
> (In reply to Chris Dumez from comment #43)
> > (In reply to Chris Dumez from comment #42)
> > > (In reply to Truitt Savell from comment #41)
> > > > The changes in https://trac.webkit.org/changeset/243094/webkit
> > > > 
> > > > appears to have broken media/track/track-in-band-style.html
> > > > 
> > > > History:
> > > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > > > html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-style.html
> > > > 
> > > > Diff:
> > > > ---
> > > > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > > > track/track-in-band-style-expected.txt
> > > > +++
> > > > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > > > track/track-in-band-style-actual.txt
> > > > @@ -14,6 +14,6 @@
> > > >  
> > > >  ** Test current cue colors
> > > >  EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255,
> > > > 255)') OK
> > > > -EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
> > > > +EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'),
> > > > OBSERVED 'rgba(0, 0, 0, 0)' FAIL
> > > >  END OF TEST
> > > >  
> > > > 
> > > > I reproduced this on 243094 and saw no failures when running it on 243092.
> > > > 243093 is a versioning patch.
> > > 
> > > EWS was green and the tests were passing locally so this is odd. I am
> > > investigating...
> > 
> > It does not reproduce locally. Also note that 'rgb(0, 0, 0)') and 'rgba(0,
> > 0, 0, 0)' are not terribly different. They are still the same color.
> 
> This seems to rely on Color::cssText() so Color::isOpaque() returns false
> and we use rgba() instead of rgb() format.

Ok, so if I run this test in a loop 100 times with -gf, then it fails some of the runs. This test is flaky. Given the nature of my change, I think I merely impacted the timing and made the test flakier.
Now that I can reproduce, I will see if I can deflake the test.

[Chris Dumez]

(In reply to Chris Dumez from comment #45)
> (In reply to Chris Dumez from comment #44)
> > (In reply to Chris Dumez from comment #43)
> > > (In reply to Chris Dumez from comment #42)
> > > > (In reply to Truitt Savell from comment #41)
> > > > > The changes in https://trac.webkit.org/changeset/243094/webkit
> > > > > 
> > > > > appears to have broken media/track/track-in-band-style.html
> > > > > 
> > > > > History:
> > > > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > > > > html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-style.html
> > > > > 
> > > > > Diff:
> > > > > ---
> > > > > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > > > > track/track-in-band-style-expected.txt
> > > > > +++
> > > > > /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/
> > > > > track/track-in-band-style-actual.txt
> > > > > @@ -14,6 +14,6 @@
> > > > >  
> > > > >  ** Test current cue colors
> > > > >  EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255,
> > > > > 255)') OK
> > > > > -EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
> > > > > +EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'),
> > > > > OBSERVED 'rgba(0, 0, 0, 0)' FAIL
> > > > >  END OF TEST
> > > > >  
> > > > > 
> > > > > I reproduced this on 243094 and saw no failures when running it on 243092.
> > > > > 243093 is a versioning patch.
> > > > 
> > > > EWS was green and the tests were passing locally so this is odd. I am
> > > > investigating...
> > > 
> > > It does not reproduce locally. Also note that 'rgb(0, 0, 0)') and 'rgba(0,
> > > 0, 0, 0)' are not terribly different. They are still the same color.
> > 
> > This seems to rely on Color::cssText() so Color::isOpaque() returns false
> > and we use rgba() instead of rgb() format.
> 
> Ok, so if I run this test in a loop 100 times with -gf, then it fails some
> of the runs. This test is flaky. Given the nature of my change, I think I
> merely impacted the timing and made the test flakier.
> Now that I can reproduce, I will see if I can deflake the test.

Tracking this flaky test via https://bugs.webkit.org/show_bug.cgi?id=195922.