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

Status: VERIFIED FIXED

(Core :: Networking: Cache, defect)

Description

[Jason Smith [:jsmith]]

Build - 1/6/2014 Buri 1.3

STR

1. In marketplace, download wikipedia
2. After the download completes, kill the network connection
3. Open wikipedia

Expected

Wikipedia should load using offline cached resources from appcache.

Actual

An offline error is thrown indicating that a network connection is needed.

Comment 1

[Carmen Jimenez Cabezas]

Sorry for the delay in updating this. 

The problem is that when the cache is preloaded (at install time) it's loaded with a ClientID generated in [1] which has the form:

https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache%231028+f|0000001389960273|1

but when the cache is actually used, the ClientID has the form:

HTTP~1028~0

which is generated at [2] (called from [3]). So nothing is even found. 

I think this is a regression introduced by bug 818667, BTW.

I could change [3] so the client ID generated is more like the one that's actually used, or the other way around. The problem is that I don't know the cache code well enough to ensure I won't break something else if I do that.

Note that it can be also that the Application Cache context isn't set correctly (it isn't set at all for what I've seen) on the window that holds the page being loaded, and that might cause also the problem (or that might fix it...). But I haven't seen any place to set that.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/cache/nsDiskCacheDeviceSQL.cpp#1325
[2] https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpHandler.cpp#1757
[3] http://mxr.mozilla.org/mozilla-central/source/uriloader/prefetch/nsOfflineCacheUpdate.cpp#1271

Comment 2

[Jason Smith [:jsmith]]

(In reply to Carmen Jimenez Cabezas from comment #1)
> Sorry for the delay in updating this. 
> 
> The problem is that when the cache is preloaded (at install time) it's
> loaded with a ClientID generated in [1] which has the form:
> 
> https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.
> appcache%231028+f|0000001389960273|1
> 
> but when the cache is actually used, the ClientID has the form:
> 
> HTTP~1028~0
> 
> which is generated at [2] (called from [3]). So nothing is even found. 
> 
> I think this is a regression introduced by bug 818667, BTW.

That can't be right - this problem hasn't existed in 1.01. bug 818667 landed in the 1.01 timeframe.

Comment 3

[Carmen Jimenez Cabezas]

You're absolutely right. Somehow I saw 2013 on the patch date instead of 2012. Too much time looking at code I guess :).

The problem is still the same, though, even if the bug wasn't introduced with that patch (clientid being one at cache fill time and a different one at cache using time). I'll keep looking on Monday to see if I can figure why or when was that changed.

I'm keeping the need-infos in case they can offer any suggestion.

Comment 4

[Honza Bambas (:mayhemer)]

There can be several reasons why wikipedia app doesn't load from the appcache:

- appcache has not been cached properly (a failure during load has happen, like a resource was not found, a resource was redirecting...) ; we have browser console logs for that on desktop (http://www.janbambas.cz/offline-cache-update-console-logs-introduced-in-firefox-19/)

- the "offline-app" permission is not set for the wikipedia app (recent change that might affect it is bug 940254, landed 10 days ago on mozilla-central)


"offline-app" permission makes the load first search cached data in an appcache storage.  During this search the correct clientid and storage to load from is automatically selected in nsHttpChannel.  You don't need to do anything if the appcache (the appcached data) is available to the process loading the app.


(In reply to Carmen Jimenez Cabezas from comment #1)
> Sorry for the delay in updating this. 
> 
> The problem is that when the cache is preloaded (at install time) it's
> loaded with a ClientID generated in [1] which has the form:
> 
> https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.
> appcache%231028+f|0000001389960273|1

Yes, this is used by the offline cache update that loads and populates the cache.

> 
> but when the cache is actually used, the ClientID has the form:
> 
> HTTP~1028~0
> 
> which is generated at [2] (called from [3]). So nothing is even found. 

This is nonsense.  [3] (nsOfflineCacheUpdate.cpp#1271) doesn't call [2] (nsHttpHandler.cpp#1757).  And [3] has nothing to do with the actual load of the app.


I presume you are on current m-c source code, then check for the following:

at http://hg.mozilla.org/mozilla-central/annotate/b66ec333bea6/netwerk/protocol/http/nsHttpChannel.cpp#l2553 check mChooseApplicationCache is set to 'true' for the app's top level page.  If not, then the "offline-app" permission is not set for the app's domain.

If it is, then check at http://hg.mozilla.org/mozilla-central/annotate/b66ec333bea6/netwerk/cache2/CacheStorage.cpp#l153 that an appcache (nsIApplicationCache) is found by call to appCacheService->ChooseApplicationCache.  If not, then the cache is not there and something failed during its load.

> 
> I think this is a regression introduced by bug 818667, BTW.
> 
> I could change [3] so the client ID generated is more like the one that's
> actually used, or the other way around. The problem is that I don't know the
> cache code well enough to ensure I won't break something else if I do that.
> 
> Note that it can be also that the Application Cache context isn't set
> correctly (it isn't set at all for what I've seen) on the window that holds
> the page being loaded, and that might cause also the problem (or that might
> fix it...). But I haven't seen any place to set that.

Please don't, as I describe above, this all is already done, all is about to work when processed properly.

> 
> [1]
> http://mxr.mozilla.org/mozilla-central/source/netwerk/cache/
> nsDiskCacheDeviceSQL.cpp#1325
> [2]
> https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/
> nsHttpHandler.cpp#1757
> [3]
> http://mxr.mozilla.org/mozilla-central/source/uriloader/prefetch/
> nsOfflineCacheUpdate.cpp#1271

Next time please refer source using the 'blame' link.  This will soon change and links will point to a different source code.

Comment 5

[Jason Smith [:jsmith]]

(In reply to Honza Bambas (:mayhemer) from comment #4)
> There can be several reasons why wikipedia app doesn't load from the
> appcache:
> 
> - appcache has not been cached properly (a failure during load has happen,
> like a resource was not found, a resource was redirecting...) ; we have
> browser console logs for that on desktop
> (http://www.janbambas.cz/offline-cache-update-console-logs-introduced-in-
> firefox-19/)

That can't be right. Wikipedia's appcache worked fine in 1.01 & 1.1, so I don't see why this would have regressed here. I've also seen this reproduce with my appcache test app (smallappcachepreload) at http://mozilla.github.io/qa-testcase-data/webapi/apps/, so I think something is more likely fundamentally broken with appcache preloading with hosted apps.

> 
> - the "offline-app" permission is not set for the wikipedia app (recent
> change that might affect it is bug 940254, landed 10 days ago on
> mozilla-central)

This wouldn't affect 1.3 specifically. However, this seems like a bad idea to enforce for apps preloading appcache anyways because an app developer already specifying an appcache path has already specified intention in the manifest to desire appcache preloading on install. There's no need to add a 2nd permission to do so to introduce compatibility risk.

Comment 6

[Matthew Vaughan]

This issue started reproducing on the 08/17/13 1.2 build.

- Works -
Device: Buri v1.2 MOZ RIL
BuildID: 20130816040203
Gaia: be95c3b6757290fff4fdde7a2b73c68f599131a1
Gecko: 6f265af4e3d8
Version: 26.0a1
Firmware Version: V1.2-device.cfg

- Broken -
Device: Buri v1.2 MOZ RIL
BuildID: 20130817040203
Gaia: 2cad0ff0180e0b33ac49aed3030949e6edbda95d
Gecko: 8ad1e4c838c8
Version: 26.0a1
Firmware Version: V1.2-device.cfg

Comment 7

[Jason Smith [:jsmith]]

Okay - the regression range implies this isn't related to the offline error UI rewrite.

Comment 8

[Jason Smith [:jsmith]]

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6f265af4e3d8&tochange=8ad1e4c838c8

Comment 9

[Jason Smith [:jsmith]]

This is a regression from bug 892488.

Comment 10

[Gregor Wagner [:gwagner]]

Honza, if this is a regression from bug 892488, any ideas how to proceed here?

Comment 11

[Honza Bambas (:mayhemer)]

There is no response to my comment 4 so I'll try to reproduce locally with m-c and check where the problem is.

Comment 12

[Honza Bambas (:mayhemer)]

So, in the local part of the profile I don't see anything preloaded in OfflineCache dir when I start Wikipedia app for the first time while being offline.  Seems like preloading the offline cache fails when installing the app (and being online that time) or it is not even happening.  Does the app specify appcache manifest reference in the app manifest?  If not, I think it's expected to first launch when online.  I tried to launch the app when online and offline cache has populated, so the app works when offline, but only for the first launch must be online.  


So, if there is a bug then it's (as usual) lack of error reporting or lack of knowledge on the developer side to properly refer the appcache manifest.


I think this is either invalid or is a bug in error reporting or a general concept bug: don't expect apps that use offline cache always work offline unless we somehow force the developers to properly refer appcache manifests in the respective app manifests.

Comment 13

[Jason Smith [:jsmith]]

(In reply to Honza Bambas (:mayhemer) from comment #12)
> So, in the local part of the profile I don't see anything preloaded in
> OfflineCache dir when I start Wikipedia app for the first time while being
> offline.  Seems like preloading the offline cache fails when installing the
> app (and being online that time) or it is not even happening.  Does the app
> specify appcache manifest reference in the app manifest?  If not, I think
> it's expected to first launch when online.  I tried to launch the app when
> online and offline cache has populated, so the app works when offline, but
> only for the first launch must be online.  

Yes - the app does specify an appcache path in the manifest.

{
  "version": "0.3",
  "name": "Wikipedia",
  "description": "Wikipedia Mobile for Firefox OS",
  "launch_path": "/WikipediaMobileFirefoxOS/",
  "appcache_path": "/WikipediaMobileFirefoxOS/app.appcache",
  "icons": {
    "16": "/WikipediaMobileFirefoxOS/image/W_launchicon_16_1101.png",
    "48": "/WikipediaMobileFirefoxOS/image/W_launchicon_48_1101.png",
    "128": "/WikipediaMobileFirefoxOS/image/W_launchicon_128_1101.png"
  },
  "developer": {
    "name": "Wikimedia Foundation",
    "url": "http://wikipedia.org"
  },
  "installs_allowed_from": ["*"],
  "locales": {},
  "default_locale": "en"
}

So I think what's not working here is preloading the appcache at install time.

> 
> 
> So, if there is a bug then it's (as usual) lack of error reporting or lack
> of knowledge on the developer side to properly refer the appcache manifest.

No, that's not right. This reproduces with a simplified test app for testing a hosted app with appcache preloading. There's also a clear regression range that indicates this is a problem a gecko, not the app itself.

> 
> 
> I think this is either invalid or is a bug in error reporting or a general
> concept bug: don't expect apps that use offline cache always work offline
> unless we somehow force the developers to properly refer appcache manifests
> in the respective app manifests.

This is incorrect. A clear regression range & a reduced test case have already disproved that this is an app developer bug.

Comment 14

[Jason Smith [:jsmith]]

Putting needinfo on Fabrice to see why the DOM Apps API is failing to preload appcache anymore.

Comment 15

[Antonio Manuel Amaya Calvo (:amac)]

I'm building a new build with added logs now, but with a Mozilla Central build from last week (from the 17th), the cache *is* populated. To test this I:

* Started the wifi connection
* Connected to the Mozilla Marketplace
* Installed the wikipedia app
* Waited till I got the 'Wikipedia installed correctly message (at that time I also got:

E/GeckoConsole(  136): Offline cache update done, URL=https://bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache

On the logcat)
* Then went into plane mode
* Tried to start the Wikipedia

And I got an error (this apps needs to be online blablabla). But:

* on /data/local/OfflineCache/index.sqlite:

sqlite> select * from moz_cache where key like '%index%';
https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache%231032+f|0000001390650325|1|https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html|request-method|0|10055|1|1390650326000000|1390650327000000|1391217956000000|2
https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache%231032+f|0000001390650325|1|https://bits.wikimedia.org/WikipediaMobileFirefoxOS/js/lib/lawnchair/adapters/indexed-db.js|request-method|0|6630|1|1390650331000000|1390650333000000|1391217963000000|2

sqlite> select * from moz_cache_namespaces;
https%3A//marketplace.firefox.com/manifest.appcache%3Frepo%3Dfireplace%231026+f|0000001390650305|0|https://marketplace.firefox.com/manifest.appcache?repo=fireplace||1
https%3A//marketplace.firefox.com/manifest.appcache%3Frepo%3Dfireplace%231026+f|0000001390650305|0|||1
https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache%231032+f|0000001390650325|1|||1


* With this file (and I also checked that the directories 0-F *had* the files on them), I still got the error I said before when I tried to start the wikipedia while being offline. 

* If I then go online, start the wikipedia, go offline, and start it again, it starts correctly

I can upload the index.sqlite now in case it's of any help, but reproducing this should be straightforward.

Comment 16

[Antonio Manuel Amaya Calvo (:amac)]

TL;DR: From the logs, it seems that Wikipedia works after first launch not because the offline cache is populated then, but because it caches the content to the disk cache. The Offline cache is populated from the start but it's not used, as Carmen said in comment 1.



BTW, the cache manifest for the wikipedia have at least one problem, I think. The launch URI https://bits.wikimedia.org/WikipediaMobileFirefoxOS/ is not cached at all. But even after changing that to https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html, when trying to launch the app while offline I get:

I/Gecko   (  135): nsHttpChannel::BeginConnect [this=0x490f3000]
I/Gecko   (  135): host=bits.wikimedia.org port=-1
I/Gecko   (  135): uri=https://bits.wikimedia.org/WikipediaMobileFirefoxOS/
I/Gecko   (  135): nsHttpChannel::BeginConnect [this=0x490f3000] prefetching
I/Gecko   (  135): nsHttpChannel::Connect [this=0x490f3000]
I/Gecko   (  135): nsHttpChannel::OpenCacheEntry [this=0x490f3000]
I/Gecko   (  135): Opening entry for session 0x4916c9a0, key https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html, mode 1, blocking 1
I/Gecko   (  135): Created request 0x44799d60
I/Gecko   (  135): Activate entry for request 0x44799d60
I/Gecko   (  135): Active entry for request 0x44799d60 is 0x0
I/Gecko   (  135): nsOfflineCacheDevice::FindEntry [key=HTTP~1027~0:https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html]
I/Gecko   (  135): nsOffLineCacheDevice::FindEntry cid: HTTP~1027~0 key: https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html
I/Gecko   (  135): Device search for request 0x44799d60 returned 0x0
I/Gecko   (  135): nsHttpChannel::OnLookupComplete [this=0x490f3000] prefetch complete: success status[0x0]
I/Gecko   (  135): nsHttpChannel::OnCacheEntryAvailable [this=0x490f3000 entry=0x0 new=0 appcache=0x0 status=804b003d]

As you can see, it's looking with cid: HTTP~1027~0, which isn't stored on the offline cache at all. After I start the app for the first time, then go offline, and for good measure rebooted the phone, I got:

I/Gecko   (  136): nsHttpChannel::BeginConnect [this=0x472d0800]
I/Gecko   (  136): host=bits.wikimedia.org port=-1
I/Gecko   (  136): uri=https://bits.wikimedia.org/WikipediaMobileFirefoxOS/
I/Gecko   (  136): nsHttpChannel::BeginConnect [this=0x472d0800] prefetching
I/Gecko   (  136): nsHttpChannel::Connect [this=0x472d0800]
I/Gecko   (  136): nsHttpChannel::OpenCacheEntry [this=0x472d0800]
I/Gecko   (  136): Opening entry for session 0x46279c20, key https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html, mode 1, blocking 1
I/Gecko   (  136): Created request 0x44ed26a0
I/Gecko   (  136): Activate entry for request 0x44ed26a0
I/Gecko   (  136): Active entry for request 0x44ed26a0 is 0x0
I/Gecko   (  136): CACHE: ReadDiskCacheEntry [b75c6f5e]
I/Gecko   (  136): nsHttpChannel::OnCacheEntry LOG 2.4: 0x00000000
I/Gecko   (  136): CACHE: nsDiskCacheBlockFile::Read [this=0x469c1878] returned 8192 / 8192 bytes
I/Gecko   (  136): Device search for request 0x44ed26a0 returned 0x464e25c0
I/Gecko   (  136): Added entry 0x464e25c0 to mActiveEntries
I/Gecko   (  136):   descriptor 0x4361cd60 created for request 0x44ed26a0 on entry 0x464e25c0
I/Gecko   (  136): nsHttpChannel::OnCacheEntryCheck enter [channel=0x472d0800 entry=0x46279c20]
I/Gecko   (  136): Opened cache input stream [channel=0x472d0800, wrapper=0x449e06b0, transport=0x45bb5780, stream=0x46282180]
I/Gecko   (  136): nsHttpChannel::OnCacheEntryAvailable [this=0x472d0800 entry=0x46279c20 new=0 appcache=0x0 status=0]
I/Gecko   (  136): nsHttpChannel::ReadFromCache [this=0x472d0800] Using cached copy of: https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html
I/Gecko   (  136): CACHE: disk OpenInputStreamForEntry [0x464e25c0 1 0]
I/Gecko   (  136): CACHE: ReadDataCacheBlocks [b75c6f5e size=10055]
I/Gecko   (  136): CACHE: nsDiskCacheBlockFile::Read [this=0x469c1878] returned 10055 / 10055 bytes
I/Gecko   (  136): nsInputStreamWrapper::LazyInit [entry=0x4361cd60, wrapper=0x46282180, mInput=0x44ed2af0, rv=0]
I/Gecko   (  136): CACHE: nsDiskCacheInputStream::Read [stream=0x44ed2af0, count=10055d, byteRead=10055d] 
I/Gecko   (  136): nsInputStreamWrapper::Read [entry=0x4361cd60, wrapper=0x46282180, mInput=0x44ed2af0, rv=0]
I/Gecko   (  136): CACHE: nsDiskCacheInputStream::Read [stream=0x44ed2af0] stream at end of file
I/Gecko   (  136): nsInputStreamWrapper::Read [entry=0x4361cd60, wrapper=0x46282180, mInput=0x44ed2af0, rv=0]
I/Gecko   (  136): nsHttpChannel::OnStartRequest [this=0x472d0800 request=0x45b9ca60 status=0]
I/Gecko   (  136):   calling mListener->OnStartRequest
I/Gecko   (  136): HttpChannelParent::OnStartRequest [this=0x446f4500]
I/Gecko   (  136): nsHttpChannel::OnDataAvailable [this=0x472d0800 request=0x45b9ca60 offset=0 count=10055]
I/Gecko   (  136): sending status notification [this=0x472d0800 status=804b0008 progress=10055/10055]
I/Gecko   (  428): HttpChannelChild::RecvOnStartRequest [this=0x43fc0400]
I/Gecko   (  136): HttpChannelParent::OnDataAvailable [this=0x446f4500]
I/Gecko   (  136): nsHttpChannel::OnStopRequest [this=0x472d0800 request=0x45b9ca60 status=0]
I/Gecko   (  136):   calling OnStopRequest
I/Gecko   (  136): HttpChannelParent::OnStopRequest: [this=0x446f4500 status=0]
I/Gecko   (  136): nsHttpChannel::CloseCacheEntry [this=0x472d0800] mStatus=0 mCacheEntryIsWriteOnly=0
I/Gecko   (  428): HttpChannelChild::OnTransportAndData [this=0x43fc0400]
I/Gecko   (  428): HttpChannelChild::OnStopRequest [this=0x43fc0400 status=0]
I/Gecko   (  136): Deactivating entry 0x464e25c0
I/Gecko   (  136): Removed deactivated entry 0x464e25c0 from mActiveEntries
I/Gecko   (  136): CACHE: disk DeactivateEntry [0x464e25c0 b75c6f5e]
I/Gecko   (  136): nsDiskCacheDeviceDeactivateEntryEvent[0x46282340]
I/Gecko   (  136): CACHE: WriteDiskCacheEntry [b75c6f5e]
I/Gecko   (  136): CACHE: DeleteStorage [b75c6f5e 1]
I/Gecko   (  428): nsContentSink::ProcessOfflineManifest. Step 4: app.appcache
I/Gecko   (  428): nsContentSink::ProcessOfflineManifest. Step 5
I/Gecko   (  136): CACHE: UpdateRecord [b75c6f5e]
I/Gecko   (  136): CACHE: InvalidateCache
I/Gecko   (  136): CACHE: WriteCacheClean: 0



As you can't see, it *isn't* reading the OfflineCache at all. It's reading the content from the diskCache, without accessing FindEntry.

Comment 17

[Honza Bambas (:mayhemer)]

Somehow the "never email me about this bug" was turned on for me.  I completely missed the bug mail about this.

I will look at this again with the new facts.

Comment 18

[Honza Bambas (:mayhemer)]

(In reply to Antonio Manuel Amaya Calvo (:amac) from comment #16)
> I/Gecko   (  135): nsOffLineCacheDevice::FindEntry cid: HTTP~1027~0 key:
> https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html

Which code exactly produces this log?  I cannot find it anywhere in the platform.

Comment 19

[Honza Bambas (:mayhemer)]

Boris, the question for you is at the second half of this long comment.


Last time I was confused (was not aware) we are moving the OfflineCache directory from the roaming part of the profile to the local part.  I mistakenly though we were not precaching the content.  But we do.


This is actually not a regression from bug 892488, that bug just uncovered a larger problem that was always there.


I now can understand the problem.  It is that the permission is not checked correctly (and never was, even before the regression bug).  Wikipedia app doesn't lookup appcache since "offline-app" perm is _not_ allowed for only a _DOMAIN_, where this domain is "bits.wikimedia.org":

  In nsDocShell::DoURILoad when the top level loading channel is (has to be) set the appropriate flag to "choose app cache" (appCacheChannel->SetChooseApplicationCache) we check whether app cache is allowed for a code-base principal that doesn't take appid and inbrowser flag into account, at all.  nsOfflineCacheUpdateService::OfflineAppAllowedForURI does nsContentUtils::GetSecurityManager()->GetNoAppCodebasePrincipal.

Inspecting the permissions.sqlite in the app's roaming part of the profile tells me the permission is set correctly (the appid == 1001):

sqlite> select * from moz_hosts;
1|bits.wikimedia.org|offline-app|1|0|0|1001|0
2|bits.wikimedia.org|pin-app|1|0|0|1001|0
...


The check at NS_ShouldCheckAppCache used at [1] is wrong, it doesn't check for appid == 1001.


It used to work (falsely actually) because at [2] offline-apps.allow_by_default pref is at true and we have pretended that _any_ principal was allowed to do offline loads.




Boris, 

here is the thing: we need to have the principal of the loading document at DoURILoad in nsDocShell to perform the permission check against, is that possible? - I presume not..

other option would be to have the appid/inbrowser properties in hands (a bit more work, but would be a solution too).



I've inspected the document's principal that is reachable via nsHttpChannel's callbacks at the time we do AsyncOpen on it and it's not the principal we would want to check against (null appid).


[1] http://hg.mozilla.org/mozilla-central/annotate/7e79536aca0a/docshell/base/nsDocShell.cpp#l9674
[2] https://bugzilla.mozilla.org/attachment.cgi?id=790823&action=diff#a/uriloader/prefetch/nsOfflineCacheUpdateService.cpp_sec2


PS: it's fun (or sad?) that the patch for bug 892488 had to spent one full cycle to find this out...

Comment 20

[Honza Bambas (:mayhemer)]

Workaround is to set the "offline-app" permission for only the domain (with appid = 0, inbrowser = false).

Comment 21

[Boris Zbarsky [:bzbarsky]]

> we need to have the principal of the loading document at DoURILoad in nsDocShell to
> perform the permission check against

Do you mean the document that would be loaded by this network load, or the document that caused the load to start?

You have something sort of like the latter as ownerPrincipal (or aOwner if you want to ignore the sandbox bits), right?

Comment 22

[Honza Bambas (:mayhemer)]

(In reply to Boris Zbarsky [:bz] from comment #21)
> > we need to have the principal of the loading document at DoURILoad in nsDocShell to
> > perform the permission check against
> 
> Do you mean the document that would be loaded by this network load, or the
> document that caused the load to start?
> 
> You have something sort of like the latter as ownerPrincipal (or aOwner if
> you want to ignore the sandbox bits), right?


I want the document that would be loaded by this network load.  That is IMO a bit more complicated.

Comment 23

[Boris Zbarsky [:bzbarsky]]

> I want the document that would be loaded by this network load.

You can't know its principal until the load finishes, because of HTTP (and other) redirects, no?

Comment 24

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Honza Bambas (:mayhemer) from comment #18)
> (In reply to Antonio Manuel Amaya Calvo (:amac) from comment #16)
> > I/Gecko   (  135): nsOffLineCacheDevice::FindEntry cid: HTTP~1027~0 key:
> > https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html
> 
> Which code exactly produces this log?  I cannot find it anywhere in the
> platform.

Sorry about that, I added a ton of traces to follow the execution of this. I thought I had removed all my custom ones from what I copied but it must have slipped. I can tell you where I added it but I guess from your other comments that you don't need it anymore :)

Comment 25

[Jason Duell]

Honza: so is the fix here to implement the hack in comment 20, or is there something else we can do?

We need something to work here ASAP.

Comment 26

[Honza Bambas (:mayhemer)]

I'll jump on this tomorrow.

The only thing I need is to know how to get to appid and inbrowser in nsDocShell::DoURILoad, or more precisely before AsyncOpen is called on the loading channel.  Then I'm happy.  Boris?

Comment 27

[Boris Zbarsky [:bzbarsky]]

nsODocShell::GetIsInBrowserElement() and nsIDocShell::GetAppId?

Comment 28

[Honza Bambas (:mayhemer)]

Thanks, got it.  But there is another problem.  The installer must set "browser.cache.offline.parent_directory" pref to point to the OfflineCache location at the roaming part of the profile.  Otherwise at the first start the preloaded cache content is not found.

Where do I find the code that should be reliable for this?

Comment 29

[Boris Zbarsky [:bzbarsky]]

For setting a pref, or for finding the value to set the pref to?  Or for putting the pref set so it happens on install?

Comment 30

[Honza Bambas (:mayhemer)]

Attachment: v1

This is what's needed in the docshell.

Comment 31

[Honza Bambas (:mayhemer)]

(In reply to Boris Zbarsky [:bz] from comment #29)
> For setting a pref, or for finding the value to set the pref to?  Or for
> putting the pref set so it happens on install?

There is a code that is triggering the appcache preload.  I don't know where it is.  There the pref I refer to should be prefilled too.  But as I see the fresh-after-install profile dir, there are no prefs.js...

Comment 32

[Honza Bambas (:mayhemer)]

Ha! problem is elsewhere.  We are trying to move (by rename) the cache in the roaming part to the local part.  But this operation fails, so we just delete the roaming offline cache.  Hmm... I'm curious why the "rename" call fails at http://hg.mozilla.org/mozilla-central/annotate/5e0a22097bff/netwerk/cache/nsCacheService.cpp#l3159.

Comment 33

[Honza Bambas (:mayhemer)]

Note that I'm on windows.  We are failing with ERROR_ALREADY_EXISTS.  So, rename() on Win32 must move to a non existent dir.  I don't know if this is issue also for b2g (GONK based).

Comment 34

[Honza Bambas (:mayhemer)]

Comment on attachment 8368441 [details] [diff] [review]
v1

This is a full fix if rename() on GONK works to an existing dir. (See comment 32, 33)

Otherwise just remove the |aNewCacheSubdir->Create(nsIFile::DIRECTORY_TYPE, 0777);| line in nsCacheService::MoveOrRemoveDiskCache and we are done here.

Comment 35

[Honza Bambas (:mayhemer)]

Hmm.. still doesn't work locally, not sure why.  I'll look at this later (need to sleep now).

Comment 36

[Honza Bambas (:mayhemer)]

So... the problem is that the precached appcache is cached for domain only (no appid in the clientid).

Hence check on the group id (with jar) fails @ nsOfflineCacheDevice::CanUseCache.  In detail: the group is generated for the loading appid and inbrowser is different then the one found in the appcache database.

So, we have to change the code that prefills the appcache to cache it for the appid+inbrowser correctly.


......

It's absolutely unbelievable that this all has been discovered so late.  This issue uncovers (so far) 3 major bugs: bad perm check, bad cache move, bad cache prefill.

Comment 37

[Honza Bambas (:mayhemer)]

Attachment: v2 [comlpete fix, 3-in-1: prefill, move, perm check]

- dochshell: the permission check takes into accout appid and inbrowser of the loading context
- offline cache device: fixed move of the prefilled appcache data from the roaming profile to local profile (tested only on windows)
- offline cache update: simplified api to always take custom profile file as well as appid/inbrowser ; this puts appid to the offline cache database correctly

Comment 38

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8368543 [details] [diff] [review]
v2 [comlpete fix, 3-in-1: prefill, move, perm check]

Bouncing the docshell review to someone who understands our app id stuf, hopefully...

Comment 39

[Jason Duell]

Comment on attachment 8368543 [details] [diff] [review]
v2 [comlpete fix, 3-in-1: prefill, move, perm check]

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

This all looks good to me.  Thanks for figuring this out Honza!

Comment 40

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

Comment on attachment 8368543 [details] [diff] [review]
v2 [comlpete fix, 3-in-1: prefill, move, perm check]

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

r=me on the docshell and nsNetUtil changes. Let me know if you need review of any other parts.

Comment 41

[Honza Bambas (:mayhemer)]

Comment on attachment 8368543 [details] [diff] [review]
v2 [comlpete fix, 3-in-1: prefill, move, perm check]

https://hg.mozilla.org/integration/mozilla-inbound/rev/73787d39b55d

Comment 42

[Honza Bambas (:mayhemer)]

https://tbpl.mozilla.org/?tree=Try&rev=b6775ba32667

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/73787d39b55d

Comment 44

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/276c08e99d1f

Comment 45

[Jason Smith [:jsmith]]

The patch here didn't work - I'm still not getting wikipedia to load offline with appcache after installing it. I'm opening a followup.

Comment 46

[Honza Bambas (:mayhemer)]

So why are you marking this as verified?

Comment 47

[Jason Smith [:jsmith]]

(In reply to Honza Bambas (:mayhemer) from comment #46)
> So why are you marking this as verified?

It was to indicate the testing is finished here (usually that's what I mark for verified for).

Comment 48

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #47)
> (In reply to Honza Bambas (:mayhemer) from comment #46)
> > So why are you marking this as verified?
> 
> It was to indicate the testing is finished here (usually that's what I mark
> for verified for).

Or we can back out the patch here & reopen. Or do something else. Up to you.

Comment 49

[Honza Bambas (:mayhemer)]

I'd leave it resolved/fixed, but you know much more about bug verification process then I do.

Let's focus on the followup.  BTW, thanks for managing this.

Comment 50

[Antonio Manuel Amaya Calvo (:amac)]

Is it failing only on the Wikipedia or have you tested something else? Because as I said before, the cache manifest for the wikipedia have at least one problem. The launch URI https://bits.wikimedia.org/WikipediaMobileFirefoxOS/ is not cached at all. 

So they should either change the manifest.webapp to set the launch path to https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html or add https://bits.wikimedia.org/WikipediaMobileFirefoxOS/ to the cache manifest.

Comment 51

[Honza Bambas (:mayhemer)]

Antonio, for me Wikipedia works regarding this bug.  I can launch first time while cut off the network.  I've been not testing with anything else.

Comment 52

[Antonio Manuel Amaya Calvo (:amac)]

Hmm... It shouldn't work since the launch path is not cached (if you haven't launched it ever). 


Anyway, for me even after I manually change the manifest.webapp to load index.html it still fails. I get:


I/Gecko   ( 2039): nsOfflineCacheDevice::FindEntry [key=HTTP~1030~0:https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html]
I/Gecko   ( 2039): nsOffLineCacheDevice::FindEntry cid: HTTP~1030~0 key: https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html
I/Gecko   ( 2039): Device search for request 0x44612dc0 returned 0x0
I/Gecko   ( 2039): nsHttpChannel::OnCacheEntryAvailable [this=0x44d99400 entry=0x0 new=0 appcache=0x0 status=804b003d]
I/Gecko   ( 2039): nsHttpChannel::SetupTransaction [this=0x44d99400]
I/Gecko   ( 2039):   pipelining disallowed

While index.sqlite has:

amac@peregrin:~/dev/cache$ adb pull /data/local/OfflineCache/index.sqlite
644 KB/s (3092480 bytes in 4.686s)
amac@peregrin:~/dev/cache$ sqlite3 index.sqlite 
SQLite version 3.7.9 2011-11-01 00:52:41
Enter ".help" for instructions
Enter SQL statements terminated with a ";"
sqlite> select * from moz_cache where key like '%index.html%';
https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache%231030+f|0000001391791722|1|https://bits.wikimedia.org/WikipediaMobileFirefoxOS/index.html|request-method|0|9402|1|1391791722000000|1391791722000000|1391808360000000|2

Which is the same behavior that I was getting before this patch (the cache is stored with a CID of https%3A//bits.wikimedia.org/WikipediaMobileFirefoxOS/app.appcache%231030+f and then it's searched with a CID of HTTP~1030~0).

Comment 53

[Honza Bambas (:mayhemer)]

Bug 969365 - patch coming...