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

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Oleg Zasypkin [:azasypkin]]

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/

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

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

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Maybe Ehsan can also understand what's going on here. Thanks!

Comment 3

[(no longer active)]

Unfortunately I'm not very familiar with our speculative parsing code.  Boris may know the script element side of things...

Comment 4

[(no longer active)]

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.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

(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

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

FWIW everything works removing this line https://github.com/azasypkin/sw-tests/blob/master/fxos-app/sw.js#L9

Comment 7

[Boris Zbarsky [:bzbarsky]]

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...

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

(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!

Comment 9

[Boris Zbarsky [: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...

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

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

Comment 11

[Boris Zbarsky [: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?

Comment 12

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

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

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/09e9d33f1186

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/09e9d33f1186

Comment 15

[Boris Zbarsky [:bzbarsky]]

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)?

Comment 16

[Josh Matthews [:jdm]]

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.

Comment 17

[Boris Zbarsky [:bzbarsky]]

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.

Comment 18

[Noemí Freire (:noemi)]

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

Comment 19

[Josh Matthews [:jdm]]

Jonas, can app URLs ever be used for content which must be downloaded from a remote source?

Comment 20

[Fernando Jiménez Moreno [:ferjm]]

(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.

Comment 21

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.