Offlined content does not work for apps on home screen

• Component: Service Workers
• Status: RESOLVED FIXED

[Thomas Steiner]

Created attachment 332208 [details]
Screenshot of failing attempt to load an offlined app from home screen in airplane mode

Steps to reproduce:

1) Go to https://airhorner.com/, reload twice to make sure the Service Worker is installed and active. 
2) Turn on airplane mode, then reload. 
3) Works as intended. 
4) Turn off airplane mode again. 
5) Add the app to the home screen. 
6) Turn on airplane mode again. 
7) Try launching the app from the home screen.
8) This fails. 

Even if after 5) you launch the app a couple of times from the home screen while online and only then launch it again in airplane mode, it doesn’t work.

Tested on: Mozilla/5.0 (iPhone; CPU iPhone OS 11_3 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Mobile/15E148 Safari/604.1

[Radar WebKit Bug Importer]

<rdar://problem/36843906>

[Chris Dumez]

Created attachment 332383 [details]
WIP Patch

[Chris Dumez]

Created attachment 332387 [details]
WIP Patch

[Chris Dumez]

Will write an API test shortly.

[Chris Dumez]

Created attachment 332403 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:238:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:242:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:242:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:243:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:246:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:248:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:249:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:259:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:270:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:273:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:280:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 11 in 6 files


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

[Simon Fraser (smfr)]

Comment on attachment 332403 [details]
Patch

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

> Source/WebCore/ChangeLog:13
> +        We know initialize the registrations' active worker as soon as we load

know -> now

[Chris Dumez]

Created attachment 332411 [details]
Patch

[EWS Watchlist]

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


ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:238:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:242:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:242:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:243:  One line control clauses should not use braces.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:246:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:248:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:249:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:259:  Missing spaces around /  [whitespace/operators] [3]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:270:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:273:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:280:  Multi-line string ("...") found.  This lint script doesn't do well with such strings, and may give bogus warnings.  They're ugly and unnecessary, and you should use concatenation instead".  [readability/multiline_string] [5]
Total errors found: 11 in 6 files


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

[youenn fablet]

Comment on attachment 332411 [details]
Patch

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

> Source/WebCore/workers/service/server/SWServer.cpp:142
> +    auto registrationPtr = registration.get();

auto& registrationRef?

[Chris Dumez]

Comment on attachment 332411 [details]
Patch

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

>> Source/WebCore/workers/service/server/SWServer.cpp:142
>> +    auto registrationPtr = registration.get();
> 
> auto& registrationRef?

I think we use this Ptr pattern a lot.

[WebKit Commit Bot]

Comment on attachment 332411 [details]
Patch

Clearing flags on attachment: 332411

Committed r227696: <https://trac.webkit.org/changeset/227696>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Thomas Steiner]

I am still seeing this issue on Beta 6 (Build 15E5216a). Any info if the landed patches make it into any of the upcoming Betas?

[Chris Dumez]

(In reply to Thomas Steiner from comment #14)
> I am still seeing this issue on Beta 6 (Build 15E5216a). Any info if the
> landed patches make it into any of the upcoming Betas?

The fix for this bug went into beta 3 iirc. I have tested airhorner many times from home screen and it was working.

[Chris Dumez]

(In reply to Chris Dumez from comment #15)
> (In reply to Thomas Steiner from comment #14)
> > I am still seeing this issue on Beta 6 (Build 15E5216a). Any info if the
> > landed patches make it into any of the upcoming Betas?
> 
> The fix for this bug went into beta 3 iirc. I have tested airhorner many
> times from home screen and it was working.

I have just tried again on the latest beta the exact steps described in this bug's original description. It works for me.

I do see a popup complaining about airplane mode being on once in a while. But tapping OK, I can see and use the app offline. I think we investigated this and the popup is showing due to the app doing analytics and trying to go to the network.

[Thomas Steiner]

I can confirm this works for Airhorner.

However, when I test with trivago.com, it doesn't. Same repro steps as in the original report, just with https://www.trivago.com as the URL.

[Ryosuke Niwa]

(In reply to Thomas Steiner from comment #17)
> I can confirm this works for Airhorner.
> 
> However, when I test with trivago.com, it doesn't. Same repro steps as in
> the original report, just with https://www.trivago.com as the URL.

Tracking this issue in https://bugs.webkit.org/show_bug.cgi?id=183800.