Bug 188139 – REGRESSION (r231107): MoviStar+ launches to a blank black screen

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

[Daniel Bates]

Steps to reproduce:

1. Download and install MoviStar+ from the Spain App Store. To do this from the US you will need a billing address in Spain and need to perform the following:
    A. Open Settings > General > Language & Region. Tap Region and pick Spain.
    B. Visit <https://appleid.apple.com/account#!&page=create> and create a new Apple ID and choose Spain as your country.
    C. Open Settings > iTunes & App Stores and sign in with the Apple ID you created in (B). If prompted to enter payment details and billing address select None and enter your billing address, respectively.
    D. Open the App Store.

2. Open MoviStar+.

Then after the initial loading screen you will see a blank black screen.

[Daniel Bates]

<rdar://problem/41907319>

[Daniel Bates]

Created attachment 346095 [details]
Patch

[Brent Fulgham]

Comment on attachment 346095 [details]
Patch

r=me.

[EWS Watchlist]

Comment on attachment 346095 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.html
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.worker.html

[EWS Watchlist]

Created attachment 346104 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Daniel Bates]

Created attachment 346105 [details]
Patch and layout test

Added a layout test to ensure that setting is propagated to both the WebKit1 and WebKit2 code paths. Ideally we would use a TestWebKitAPI test that registers a custom protocol handler and loads content that makes a CORS preflight request to its custom protocol.

[EWS Watchlist]

Comment on attachment 346105 [details]
Patch and layout test

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

New failing tests:
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.html
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.worker.html

[EWS Watchlist]

Created attachment 346116 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Daniel Bates]

Created attachment 346119 [details]
Patch and layout tests

Add platform-specific results for OS X Yosemite, macOS Sierra, macOS High Sierra and iOS 11 that use the CORS behavior before <http://trac.webkit.org/changeset/231107>.

[Brent Fulgham]

Comment on attachment 346119 [details]
Patch and layout tests

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

I think this patch looks good, but it might still cause imported platform Fetch API tests to fail.

> Source/WebKit/NetworkProcess/NetworkLoadChecker.h:137
> +    bool m_shouldCaptureExtraNetworkLoadMetrics;

We did you stop initializing this value? Is it because it is always set by the constructor? It seems like it would be better to always set them to a known state, in case someone makes a constructor overload in the future that doesn't assign a value here.

[EWS Watchlist]

Comment on attachment 346119 [details]
Patch and layout tests

Attachment 346119 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8705153

New failing tests:
http/tests/xmlhttprequest/about-put-allowCORSPreflightRequestsToAnyScheme.html

[EWS Watchlist]

Created attachment 346131 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[EWS Watchlist]

Comment on attachment 346119 [details]
Patch and layout tests

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

New failing tests:
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.html
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.worker.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 346119 [details]
Patch and layout tests

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

New failing tests:
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.html
imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.worker.html

[EWS Watchlist]

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

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

[Alex Christensen]

Comment on attachment 346119 [details]
Patch and layout tests

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

> LayoutTests/ChangeLog:26
> +        * platform/mac-yosemite/imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any-expected.txt: Added.
> +        * platform/mac-yosemite/imported/w3c/web-platform-tests/fetch/api/basic/scheme-about.any.worker-expected.txt: Added.

These aren't needed after http://trac.webkit.org/r234435

[Alex Christensen]

Comment on attachment 346119 [details]
Patch and layout tests

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

> Source/WebCore/page/Settings.yaml:763
> +allowCORSPreflightRequestsToAnyScheme:
> +  initial: false

I'll bet this is being reset to false every time a test runs, leading to the test failures if the affected tests aren't the first run by DRT/WKTR

[Alex Christensen]

https://trac.webkit.org/changeset/219847/webkit might be related to the failures were seeing.

[Alex Christensen]

I think we should add a bundle check, too, so we can remove this hack soon.

[Alex Christensen]

Created attachment 346247 [details]
Patch

[Daniel Bates]

Comment on attachment 346247 [details]
Patch

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

> Source/WebCore/loader/DocumentThreadableLoader.cpp:188
> +    bool needsPreflightQuirk = IOSApplication::isMoviStarPlus() && applicationSDKVersion() < DYLD_IOS_VERSION_12_0 && (m_options.preflightPolicy == PreflightPolicy::Consider || m_options.preflightPolicy == PreflightPolicy::Force);

How did you come to the decision to limit this change only to MoviStar? This issue affects all apps. It seems weird to give preferential treatment to MoviStar and will give a bad customer experience for other Mac and iOS apps affected by r231107.

[Brent Fulgham]

Comment on attachment 346247 [details]
Patch

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

Let’s take this change as is, and revisit for other impacted client applications if any are identified,

>> Source/WebCore/loader/DocumentThreadableLoader.cpp:188
>> +    bool needsPreflightQuirk = IOSApplication::isMoviStarPlus() && applicationSDKVersion() < DYLD_IOS_VERSION_12_0 && (m_options.preflightPolicy == PreflightPolicy::Consider || m_options.preflightPolicy == PreflightPolicy::Force);
> 
> How did you come to the decision to limit this change only to MoviStar? This issue affects all apps. It seems weird to give preferential treatment to MoviStar and will give a bad customer experience for other Mac and iOS apps affected by r231107.

Do we know of any other clients impacted by this? Alex and I talked about it, and would like to be able to remove this workaround code once MoviStar (and any others) have updated.

[Alex Christensen]

Created attachment 346255 [details]
Patch

[Alex Christensen]

http://trac.webkit.org/r234447

(In reply to Daniel Bates from comment #22)
> Comment on attachment 346247 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=346247&action=review
> 
> > Source/WebCore/loader/DocumentThreadableLoader.cpp:188
> > +    bool needsPreflightQuirk = IOSApplication::isMoviStarPlus() && applicationSDKVersion() < DYLD_IOS_VERSION_12_0 && (m_options.preflightPolicy == PreflightPolicy::Consider || m_options.preflightPolicy == PreflightPolicy::Force);
> 
> How did you come to the decision to limit this change only to MoviStar? This
> issue affects all apps. It seems weird to give preferential treatment to
> MoviStar and will give a bad customer experience for other Mac and iOS apps
> affected by r231107.

I agree with Dan. It’s not clear how r231107 benefits folks who use apps linked before iOS 12, but it’s clear that r231107 carries risk to those people.

[Alex Christensen]

(In reply to mitz from comment #26)
I think this is measurably better than nothing.  Either of the Dans are welcome to improve upon this by the integration deadline.