749029 - Apps cannot operate offline right now - Offline cache cannot be used

Status: VERIFIED FIXED

(Firefox Graveyard :: Webapp Runtime, defect, P1)

Description

[Jason Smith [:jsmith]]

Steps:

1. Install an app that uses offline cache
2. Make sure offline cache gets populated while using the application
3. Turn off your internet connection
4. Continue using the application

Expected:

The application parts that utilize offline cache should still continue to work without an internet connection.

Actual:

Offline cache was not read - Connection failures get thrown (connection reset while page was loading, could not connect).

Example scenario to test this with a real app:

1. Install lanyrd mobile (https://marketplace.mozilla.org/en-US/app/lanyrd-mobile/?src=search)
2. Login with a twitter account
3. Look at your events
4. Shut off the internet connection
5. Try using the application

Comment 1

[Jason Smith [:jsmith]]

Attachment: Lanyrd Mobile Example

Comment 3

[Jason Smith [:jsmith]]

Andy and I have both confirmed this be a problem (see the bug this is a duped against, bug 754320 for an example). This is a big problem - We're advertising this in our product vision, but we apparently aren't supporting offline mode.

Comment 4

[Jason Smith [:jsmith]]

Flagging uiwanted - we need a user interface to prompt the user to allow the app to store data for offline use.

Comment 5

[Jennifer Arguello :ticachica]

UX recommendation is to use the native notification system. 

UX will put a mock up in this bug to show this.

Engineering needs to estimate the work for this.

Comment 6

[Potch [:potch]]

Why are we asking permission to use applicationCache? Permission should be implicit for an app a users installs. (Fairly certain this is the case on b2g)

Comment 7

[Jason Smith [:jsmith]]

(In reply to Potch [:potch] from comment #6)
> Why are we asking permission to use applicationCache? Permission should be
> implicit for an app a users installs. (Fairly certain this is the case on
> b2g)

True (https://wiki.mozilla.org/Apps/Security/Permissions). Desktop firefox currently does prompt the user to do this, so it's likely it's being implemented incorrectly there as well given this specification. We could either fix the problem in the core (better long term route), or work around the issue in the short-term in the front-end based on how firefox does it (although I'm against using short-term hacks unless it's absolutely necessary).

Jonas - Thoughts? Should we resolve this problem in the underlying core?

Comment 8

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

In the context of apps we should definitely not be asking for permission to use the appcache. This will be handled once we implement the automatic downloading of appcache resources during application install, which I don't think has been implemented yet.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Jonas Sicking (:sicking) from comment #8)
> In the context of apps we should definitely not be asking for permission to
> use the appcache. This will be handled once we implement the automatic
> downloading of appcache resources during application install, which I don't
> think has been implemented yet.

Gotcha. Sounds like bug 752705. Should we duplicate this bug against that one, since they target the same underlying problem? Or is this bug depend on the implementation of bug 752705?

Comment 10

[Jason Smith [:jsmith]]

Actually, I'll mark this is a depending on that bug (I think that makes more sense), unless there's disagreement on that.

Comment 11

[Jason Smith [:jsmith]]

FYI to Brian Dils - We no longer need a UI for this.

Comment 13

[Myk Melez [:myk] [@mykmelez]]

Jonas, Honza: in case the appcache changes don't land in time for Firefox 15, is there a fix we can land for just this bug?

Comment 14

[Honza Bambas (:mayhemer)]

As I understand, you lack "offline-app" permission being set to ALLOW for an app's domain.  I don't think I'm the right person to ask anything to do about that in particular, but it is fairly easy to do IMO.

If this is blocked on bug 756620, then my part (allow of preload from the firefox process, bug 753990) is mostly done - a green-try patch is waiting for second review from Michal.

Olli's bug 756131 has already been landed.

Comment 15

[Bill Walker [:bwalker] [@wfwalker]]

If I understand Jonas correctly, this bug is subsumed by the work associated with bug 756620?

Comment 16

[:Felipe Gomes (needinfo for replies!)]

No, this bug is also needed for bug 756620. Even with the appcache loaded during install, the webapp still needs its permission set to use it.

I believe that what is needed here is just to call Services.perms.addPermission in webapp.js before the app URL is loaded, but I don't know if there are other settings that need to be configured to allow appcache to work  (e.g. what controls the max cache size? is it a fixed value or something that needs to be set)

Comment 17

[Honza Bambas (:mayhemer)]

(In reply to Felipe Gomes (:felipe) from comment #16)
> I believe that what is needed here is just to call
> Services.perms.addPermission in webapp.js before the app URL is loaded

Yes.  Or on some other place, or it can be set as part of the preload process by Firefox.

> , but
> I don't know if there are other settings that need to be configured to allow
> appcache to work  (e.g. what controls the max cache size? is it a fixed
> value or something that needs to be set)

There is a default value for this and AFAIK it is 512MB.

Comment 18

[Myk Melez [:myk] [@mykmelez]]

Attachment: work in progress

Something like this should work, but I haven't tested it yet.

Comment 19

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: set appcache permissions appropriately

Here's a patch that sets the appropriate permissions for the app to be able to use appcache.  According to Jonas, the permissions are pin-app and offline-app.

Ideally, we'd do this at installation time.  But that would require creating the profile on installation and training the permissions manager to set permissions in it.  So this patch sets the permissions on firstrun, which is also when the platform creates the profile.

Tested with the Lanyrd app, which successfully cached events I tracked and showed me information about them (including information I had never looked at) when I went offline after installing and running the app using a version of Firefox built with this patch.  I also confirmed that the appropriate permissions are set in permissions.sqlite within the app's profile directory (and that webapprt.firstrun is set appropriately in the profile's prefs.js file).

Comment 20

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 628395 [details] [diff] [review]
patch v1: set appcache permissions appropriately

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

::: webapprt/CommandLineHandler.js
@@ +56,5 @@
> +
> +    // Now that we've set the appropriate permissions, twiddle the firstrun flag
> +    // so we don't try to do so again.
> +    Services.prefs.setBoolPref("webapprt.firstrun", true);
> +  }

Hmm except for the Webapps.jsm import which might need to happen super early in this component (or not, but we don't need to experiment changing it here), I believe that all the start-up code being added could happen in the global scope of webapp.js, and that would feel less hackish than adding it here in the the component code..
or is there a reason you thought about that webapp.js wouldn't work?

Comment 21

[Myk Melez [:myk] [@mykmelez]]

(In reply to Felipe Gomes (:felipe) from comment #20)
> Hmm except for the Webapps.jsm import which might need to happen super early
> in this component (or not, but we don't need to experiment changing it
> here), I believe that all the start-up code being added could happen in the
> global scope of webapp.js, and that would feel less hackish than adding it
> here in the the component code..
> or is there a reason you thought about that webapp.js wouldn't work?

To my mind, they're equally hackish, as neither context is intended for such work.

Ideally, there'd be platform support for registering code that should be executed on firstrun.  Failing that, the code might be better off in an XPCOM component that registers with the category manager for instantiation on startup.  Except that it isn't reusable by the rest of the codebase the way other XPCOM components are, so it doesn't fit the usecases for which XPCOM is intended either.  And it's Yet Another XPCOM Component (YAXC), instead of Yay One Less XPCOM Component (YOLXC)!

I chose CommandLineHandler because we're already using it to do startup work unrelated to command-line handling (initializing the webapp registry by importing Webapps.jsm), and because startup work done in CommandLineHandler will certainly happen before the app is loaded, which needs to happen in this case.

But that latter constraint can be satisfied in webapp.js as well, so it's an option, and I'm open to it.

Comment 22

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 628395 [details] [diff] [review]
patch v1: set appcache permissions appropriately

Ok sounds good, we can always revisit this later if needed. I pointed Drew to take a look here at this bug since he is also changing where Webapps.jsm gets loaded in bug 733631.

Comment 23

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 628395 [details] [diff] [review]
patch v1: set appcache permissions appropriately

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b6e98c0317b

Comment 24

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4b6e98c0317b

Comment 25

[Jason Smith [:jsmith]]

Tested with At Work - offline mode did work. Would prefer to also test lanyrd mobile (as that's the better test here to really see if this works), but I'm blocked by bug 752666 not landing. Will verify with lanyrd mobile once bug 752666 lands.

Comment 26

[Jason Smith [:jsmith]]

Attachment: Stuck on WebRT vs. Desktop Firefox Having Prompt

Comment 27

[Jason Smith [:jsmith]]

Well, thought this was fixed, but upon doing crash testing on the web runtime, I just found out it isn't fixed in every possible case :(. See attached screenshot. Basically I loaded https://touch.betfair.com/ in the desktop web runtime. As you can see by the screenshot, the runtime is stuck, likely waiting for a user response to a prompt that does not exist to enable offline cache. Although in the runtime's case, this prompt should have never happened in the first place given the above approach suggested in the above comments. Reopening.

Comment 28

[[:fabrice] Fabrice Desré]

this may be a indexedDB prompt, not an offline-cache prompt. You need to set a different permission for that.

Comment 29

[Jason Smith [:jsmith]]

(In reply to Fabrice Desré [:fabrice] from comment #28)
> this may be a indexedDB prompt, not an offline-cache prompt. You need to set
> a different permission for that.

Ah, sounds like bug 757678 then. I'll close this then and make a comment in that bug. Thanks for the input on this.