Closed Bug 969365 Opened 7 years ago Closed 7 years ago

Followup to bug 956930 - Launching an app preloading appcache throws an offline error, rather than using offline cached resources to render content

Categories

(Core :: Networking: Cache, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla30
blocking-b2g 1.3+
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: jsmith, Assigned: mayhemer)

References

Details

Attachments

(1 file)

Build

* Version: 1.4
* Build ID: 20140206160203
* Gaia: 3fc26ae786e3869a7ef1e23afc9807ac1b4741f2
* Gecko: d05c721ea1b0

Followup to bug 956930. We're still not getting wikipedia to render offline after installing it without a network connection.
Blocks: 956930
blocking-b2g: --- → 1.3+
I was testing on desktop/single process.  If there is something wrong, then it's the IPC part.  I'll quickly look into the code where the problem could be, but Fabrice played with this recently more then I did.

Fabric, any clue?
So, as usual...

I've made a desktop built of b2g-inbound ("the most stable branch ever") on my linux box, applied the desktop multiprocess patch (still not in the tree? :)) and when I run e.g. Settings app, then immediately after I click the mouse the parent process crashes... bug 969440.
Flags: needinfo?(fabrice)
So, I've installed the app w/o the multiprocess patch.  Then restarted with the patch applied and network cut off.  Confirming this bug is still there in multiprocess model.  However, the app loads (from appcache) when running b2g as single process (w/o the multiprocess patch).

Looking into it now.
So, the problem is here:
http://hg.mozilla.org/mozilla-central/annotate/d05c721ea1b0/netwerk/protocol/http/HttpChannelParent.cpp#l254

The same mistake as we were doing in docshell.  Here we must check against appid/inbrowser/url - best against principal as I've change it in docshell in the mother bug.
Attached patch v1Splinter Review
Tested with oop b2g desktop build on linux.
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attachment #8372405 - Flags: review?(jduell.mcbugs)
Flags: needinfo?(fabrice)
Comment on attachment 8372405 [details] [diff] [review]
v1

Review of attachment 8372405 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.

So why are we even supporting the non-appID-checking version of NS_ShouldCheckAppCache()?  Is there some use case that it's still useful for?  It seems mostly like a footgun at this point.
Attachment #8372405 - Flags: review?(jduell.mcbugs) → review+
(In reply to Jason Duell (:jduell) from comment #6)
> So why are we even supporting the non-appID-checking version of
> NS_ShouldCheckAppCache()?  Is there some use case that it's still useful
> for?  It seems mostly like a footgun at this point.

Good point, followup?
(In reply to Honza Bambas (:mayhemer) from comment #7)
> Good point, followup?

bug 969577
https://hg.mozilla.org/mozilla-central/rev/6208410bd962
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Keywords: verifyme
QA Contact: jsmith
lgtm - wikipedia works on device now on trunk
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.