1151916 - Refreshing a webpage using service worker's caches API breaks when using caches.open() or caches;

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Salvador de la Puente González [:salva]]

STR
0. Load the following custom prefs into Gaia
user_pref('dom.caches.enabled', true);
user_pref('dom.serviceWorkers.enabled', true);
user_pref('dom.serviceWorkers.testing.enabled', true);
user_pref('network.disable.ipc.security', true);

1. Tap on the search bar
2. Visit http://lodr.github.io/open-crash
3. Once the page is loaded, tap on the refresh button
4. Close the browser
5. Visit http://lodr.github.io/open-crash again
6. Once loaded, tap on refresh

Expected:
The page refreshes properly.

Actual:
The page crashes. If you observe the traces in adb logcat, you can see how the last trace is `Before open...`

Comment 1

[Salvador de la Puente González [:salva]]

Do you think this could be related with bug 1151892?

Comment 2

[Cristian Rodriguez (:crdlc)]

I have the same STR with other app only with \caches;\ in the 'fetch' handler without opening the cache:

URL: https://music.hostedweb.tid.es
Code of SW: https://music.hostedweb.tid.es/offliner-worker.js

I/Browser ( 1871): Content JS LOG: Offliner running! 
I/Browser ( 1871):     at <anonymous> (https://music.hostedweb.tid.es/offliner-worker.js:5:1)
I/Browser ( 1871): Content JS LOG: Offliner attached all listeners! 
I/Browser ( 1871):     at <anonymous> (https://music.hostedweb.tid.es/offliner-worker.js:78:1)
I/Browser ( 1871): Content JS LOG: Offliner > Fetch method 1 https://music.hostedweb.tid.es/ 
I/Browser ( 1871):     at <anonymous> (https://music.hostedweb.tid.es/offliner-worker.js:32:3)
I/Gecko   ( 1122): 
I/Gecko   ( 1122): ###!!! [Parent][MessageChannel] Error: Channel error: cannot send/recv
I/Gecko   ( 1122): 
I/Gecko   ( 1122): ############ ErrorPage.js
I/Gecko   ( 1122): [Parent 1122] WARNING: pipe error (95): Connection reset by peer: file ../../../gecko/ipc/chromium/src/chrome/common/ipc_channel_posix.cc, line 456
I/GeckoDump( 1122): Crash reporter : Can't fetch app.reportCrashes. Exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: chrome://b2g/content/shell.js :: shell_reportCrash :: line 146"  data: no]

Comment 3

[Cristian Rodriguez (:crdlc)]

Same happens from Browser or when the app has been installed as trusted hosted app

Comment 4

[Andrea Marchesini [:baku]]

bkelly knows more about cache. Moving the NI.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

I'm traveling at the moment and I don't have my flame with me.  I'll have to check next week.

Does this effect the desktop browser in e10s mode?

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

If you could get a stack trace of the crash, I could try to look at it sooner.  Sadly, I don't remember how to get a stack trace from a b2g crash.

Comment 7

[Maria Angeles Oteo (:oteo)]

Ben, I have reproduced a crash in Firefox Desktop Nightly following the steps in the description but replacing http with https in the URL that Salva posted.

I am not sure if it's exactly the same issue but just in case, it can help you. The crash report is available at: https://crash-stats.mozilla.com/report/index/e1acfe9f-b5e4-48f3-aabb-13f682150408

By the way, I also have crashes in nightly using the URL included by Cristian in comment 2.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

This is crashing on WorkerPrivate::GetPrincipalInfo():

  https://dxr.mozilla.org/mozilla-central/source/dom/workers/WorkerPrivate.h#574

Because the mInfo.mPrincipalInfo is nullptr.  This should not be possible, though, because mPrincipalInfo is always initialized in WorkerPrivate::SetPrincipal().

So it seems script is running before SetPrincipal() is called!

This is most likely fallout from bug 931249.

Comment 9

[Salvador de la Puente González [:salva]]

I think Maria already provided what you need. Clearing ni.

Comment 10

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: bz://1151916/nsm

/r/6927 - Bug 1151916 - Set worker principalinfo on cache load. r=bkelly

Pull down this commit:

hg pull -r 89f2f6e2e5d59e7a61f2b3125a286cb7d7d816eb https://reviewboard-hg.mozilla.org/gecko/

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8591086 [details]
MozReview Request: bz://1151916/nsm

https://reviewboard.mozilla.org/r/6925/#review5771

r=me with the one comment address and a test case that triggers the failure.  It seems getting a cached SW to load and accessing `self.caches` is enough of a test case.

::: dom/workers/ScriptLoader.cpp
(Diff revision 1)
> +      mWorkerPrivate->SetPrincipal(principal, loadGroup);

I think maybe we should only do this in the same success case as where we call `mWorkerPrivate->SetBaseURI()` above.  I don't think we should call `SetPrincipal()` if an error occurrs.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Nikhil, is the test proving difficult? I'd be ok doing that in a follow-up if it means we can fix this issue soon.  I think this bug is probably making it difficult to do any SW development in Nightly Firefox right now.

Comment 14

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e1b6cd71d29

Comment 15

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Test was involved but easy, just waiting on try now.

Comment 16

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/db7df4ebfaf5

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/db7df4ebfaf5

Comment 19

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Attachment: MozReview Request: Bug 1151916 - Set worker principalinfo on cache load. r=bkelly