Closed Bug 1175944 Opened 7 years ago Closed 7 years ago

Packaged app's (app://) JS files are not loaded and do not trigger "onfetch" handler

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
FxOS-S2 (10Jul)
Tracking Status
firefox42 --- fixed

People

(Reporter: azasypkin, Assigned: ferjm)

References

Details

Attachments

(1 file)

STR:

1. Install [1] as preinstalled app (to workaround bug 1173539);
2. Run it and make sure that service worker is registered;
3. Kill app with Task Manager and run again;
4. Observe log - there is no fetch requests for any of the app's JS files; CSS and HTML files triggered "onfetch" handler and loaded successfully.
5. Side effect is that app's background is yellow, but should be green (color is changed by script that is not loaded).

I don't see this issue if I run app in B2G browser via https [2];

Flame PVT mc Engineering Build:

Build Identifier: 20150618010201
Gaia git commit Info: 2015-06-18 14:31:48, bd681cee
Gecko hg commit info: a3f280b6f8d5

[1] https://github.com/azasypkin/sw-tests/tree/master/fxos-app
[2] http://azasypkin.github.io/sw-tests/fxos-app/
See Also: → 1175949
I tried to debug this issue but I get to a point where I am pretty blocked.

What I found out is that we are doing the interception of the index.html file and we are properly serving the content of this file from the service worker, but for some reason, the HTML5 parser doesn't load the app.js file.

When we *do not* serve the index.html from the service worker, I can see that we do two calls to nsScriptElement::MaybeProcessScript [1]. The first time the mDoneAddingChildren flag is false, so we bail out at [2]. The second call, that flag is true, so we progress with the processing of the script.

When we serve the index.thml from the service worker, we only make the first call to nsScriptElement::MaybeProcessScript while the mDoneAddingChildren flag is still false. For some reason, the parser doesn't retry to process the script.

Henri, I am running out of ideas here. Can you think of any reason why we aren't able to process the JS file in this case?

Thanks in advance for any help!

[1] https://mxr.mozilla.org/mozilla-central/source/dom/base/nsScriptElement.cpp#110
[2] https://mxr.mozilla.org/mozilla-central/source/dom/base/nsScriptElement.cpp#120
Flags: needinfo?(hsivonen)
Maybe Ehsan can also understand what's going on here. Thanks!
Flags: needinfo?(ehsan)
Unfortunately I'm not very familiar with our speculative parsing code.  Boris may know the script element side of things...
Flags: needinfo?(ehsan)
One thing you may want to check for is to see if loading each of the two scripts individually in a top-level browsing context works.  Also, if there is a way to test the app inside an "app context" but through http(s), it would be nice to know if the same issue happens there (to see if the nsJARChannel level interception code is at fault here.)

FWIW, the scripts in https://dxr.mozilla.org/mozilla-central/source/dom/workers/test/serviceworkers/app-protocol/controlled.html load just fine.  I can't think of a fundamental difference between that test and comment 0 though.
Flags: needinfo?(bzbarsky)
(In reply to Ehsan Akhgari (not reviewing patches, not reading bugmail, needinfo? me!) from comment #4)
> FWIW, the scripts in
> https://dxr.mozilla.org/mozilla-central/source/dom/workers/test/
> serviceworkers/app-protocol/controlled.html load just fine.  I can't think
> of a fundamental difference between that test and comment 0 though.

Thank you Ehsan. The difference between that mochitest and the test case from comment 0 is that on the mochitest controller.html is served directly from the network while on the test case index.html is being served from a service worker initiated fetch [1].

[1] https://github.com/azasypkin/sw-tests/blob/master/fxos-app/sw.js#L9
Assignee: nobody → ferjmoreno
Target Milestone: --- → FxOS-S2 (10Jul)
Status: NEW → ASSIGNED
Is there a testcase that reproduces the bug in a desktop Firefox with service workers enabled?  That would make this a lot easier to debug...
Flags: needinfo?(bzbarsky)
(In reply to Boris Zbarsky [:bz] from comment #7)
> Is there a testcase that reproduces the bug in a desktop Firefox with
> service workers enabled?  That would make this a lot easier to debug...

I am afraid that that's not possible :( This can only be reproduced with packaged apps. Installing a packaged app on desktop will make it use the WebappRT.

I can debug on b2g if you could give me any pointers about where should I be looking at.

Thanks!
Flags: needinfo?(bzbarsky)
OK.  Well, it sounds like you're getting the <script> inserted into the DOM (that first MaybeProcessScript call) but are never getting nsIScriptElement::AttemptToExecute called?

If that's correct, can you check whether you even end up in nsHtml5TreeOpExecutor::RunScript?  If not, we can start worrying about that...
Flags: needinfo?(bzbarsky)
No, after serving the index.html file from the service worker we don't get to any of these function calls, nsIScriptElement::AttemptToExecute or nsHtml5TreeOpExecutor::RunScript
Flags: needinfo?(bzbarsky)
OK.  Do you land in nsHtml5TreeBuilder::elementPopped with aName == nsHtml5Atoms::script?  That is, the block at http://hg.mozilla.org/mozilla-central/file/0b901209064c/parser/html/nsHtml5TreeBuilderCppSupplement.h#l800

If you do, do you reach the InitScript call at the end of that block?  Or do you take one of the early returns?  If one of the early returns is taken, which one?
Flags: needinfo?(bzbarsky)
Attached patch v1Splinter Review
Thank you Boris!

We were early returning at [1] because of [2]. We weren't setting the safe flag for synthesized JAR responses.

[1] https://mxr.mozilla.org/mozilla-central/source/parser/html/nsHtml5TreeBuilderCppSupplement.h#811
[2] https://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#2691
Flags: needinfo?(hsivonen)
Attachment #8629345 - Flags: review?(josh)
Attachment #8629345 - Flags: review?(josh) → review+
https://hg.mozilla.org/mozilla-central/rev/09e9d33f1186
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Shouldn't we set mIsUnsafe to false only if aContentType is on our list of jar content types (or whatever the normal conditions for setting it to false are)?
It's always marked as safe if the content comes from a local JAR file rather than a remote download, so I assumed that intercepting local content and replacing it with something else would treated the same.
OK.  And this intercept code is known to only run for local content?

Put another way, this code could use a nice comment explaining why it's safe.
Hi,

checked with today's (7/6) master build. Following the steps described in comment 0, there are currently fetch events received (side effect: the app's background is green). Please see below the corresponding traces:

I/fxos-app( 4070): Content JS LOG: Fetch event received app://fxos-app.gaiamobile.org/index.html 
I/fxos-app( 4070):     at <anonymous> (app://fxos-app.gaiamobile.org/sw.js:7:3)
I/fxos-app( 4070): Content JS LOG: Fetch event received app://fxos-app.gaiamobile.org/css/app.css 
I/fxos-app( 4070):     at <anonymous> (app://fxos-app.gaiamobile.org/sw.js:7:3)
I/fxos-app( 4070): Content JS LOG: Fetch event received app://fxos-app.gaiamobile.org/js/one-more-js-file.js 
I/fxos-app( 4070):     at <anonymous> (app://fxos-app.gaiamobile.org/sw.js:7:3)
I/fxos-app( 4070): Content JS LOG: Fetch event received app://fxos-app.gaiamobile.org/js/app.js 
I/fxos-app( 4070):     at <anonymous> (app://fxos-app.gaiamobile.org/sw.js:7:3)
I/fxos-app( 4070): Content JS LOG: One-More-JS-File-Parsed 
I/fxos-app( 4070):     at <anonymous> (app://fxos-app.gaiamobile.org/js/one-more-js-file.js:1:1)
I/fxos-app( 4070): Content JS LOG: Startup-File-Parsed 
I/fxos-app( 4070):     at <anonymous> (app://fxos-app.gaiamobile.org/js/app.js:1:1)
I/fxos-app( 4070): Content JS LOG: Registration succeeded. Scope is app://fxos-app.gaiamobile.org/ 
I/fxos-app( 4070):     at initServiceWorker/< (app://fxos-app.gaiamobile.org/js/app.js:5:5)


Environmental variables:
Flame device
Build Id: 20150706080959
Gecko: 4d5dc30
Gaia: dc6c18c
Platform version: 42.0a1
Jonas, can app URLs ever be used for content which must be downloaded from a remote source?
Flags: needinfo?(jonas)
(In reply to Josh Matthews [:jdm] from comment #19)
> Jonas, can app URLs ever be used for content which must be downloaded from a
> remote source?

AFAIK they can't.
No.

I mean, the content was downloaded at some point. But then we saved it locally, so at the time that the app:// URL is loaded the content is either available locally or the load will fail.
Flags: needinfo?(jonas)
You need to log in before you can comment on or make changes to this bug.