Closed Bug 835399 Opened 7 years ago Closed 7 years ago

Allow launching non-privileged webapps with a URL

Categories

(Firefox for Android :: Web Apps (PWAs), defect)

15 Branch
x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 21

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

Attachments

(5 files, 2 obsolete files)

WebApps typically pass a manifestURL in the WEBAPP ANdroid intent. We also support passing a launchURL too, but we map it to a manifestURL. This allows Firefox to set the correct permission level for the webapp. See bug 819037.

We want to support using a chromeless WebApp activity for any URL. These webapps would be launched with no permission or privilege, just like regular web pages, except in a chromeless view.

One use case for this feature is launching webapps from the Everything.me custom launcher.
I believe when this concept was explored for general web pages on Gaia, we concluded that this was a good idea, but we would include a "wrapper" UI at the bottom of the UI to allow minor navigation around pages. If you try out an everything.me app on Gaia, you'll see what I mean.
Firefox for Android has no "wrapper" UI. It's all (Firefox) or nothing (Web Apps). The intent though is the same.
(In reply to Mark Finkle (:mfinkle) from comment #2)
> Firefox for Android has no "wrapper" UI. It's all (Firefox) or nothing (Web
> Apps). The intent though is the same.

Right. Which might imply that we need UX input here then for if a wrapper is necessary. I believe B2G UX determined this as important as some of these everything.me apps for example operate under web page assumptions (e.g. there's a back button that exists), not app assumptions.
The good news is: Android has a back button.

But yeah, we'll look at UX too. But I am not easily going to add a UI to the chromeless webapp view. Some apps might suck.
Whiteboard: A4A?
I think we need to approach this in two separate routes

Immediate route - bring back the option of opening a non-manifest url with the webapp intent. This will allow us to make progress in the overall discussion of combining evme and gecko on Android and can be instrumental for the meetings we'll have early next week. Mark, we can even live with a dev build for that purpose.

Full solution route - following product and UX discussion we'll probably have next week, we'll be able to better define the behavior and functionality of launching an app from evme.

Makes sense?
Flags: needinfo?(mark.finkle)
This makes sense to me. We can work on the short term plan and discuss how to move forward.

This won't block A4A
Flags: needinfo?(mark.finkle)
Whiteboard: A4A?
Just some housekeeping. isAppUpdate actually returns a state/status, so this patch renames the method to clarify the code.
Assignee: nobody → mark.finkle
Attachment #709737 - Flags: review?(bnicholson)
This patch removes the use of "promises" from the code. We typically use callbacks and those would be fine in this case. Promises add more overhead than needed in this case.
Attachment #709738 - Flags: review?(bnicholson)
Attached patch patch 3: open a URL as an app (obsolete) — Splinter Review
This patch does a few things to get normal URLs to open in the WebApp.java activity. It still needs more work, but I wanted some feedback.

Summary:
* Create an activity for "WebApp" that won't conflict with "WebApps$WebApp#" 
* In WebAppRT.js, instead of following back to "" (no URL) if the URL is not an installed webapp, just try to use the URL itself.
* In browser.js, we try to open any URL given back from WebAppRT.init

These changes are enough to get a URL to open in a chromeless WebApp activity. The remaining problems include:
* We try to use "webapp0" profile because WebApp.getIndex() defaults to 0. We'll need to change this.
* You can't open more than one URL as a webapp because WebApp.java will ignore any subsequent onNewIntents. Also, if we tried to open WebApp as a new activity for each URL, we run into the bug where the OpenGL surface can only be used by one activity.

The last issue is the big one. One idea I have is to allow multiple "tabs" to be open in the chromeless WebApp activity, but no UI to switch between them. Tapping on the shortcut icon or a new intent to an existing "tab" would be the way to cause a "tab" to become visible. It's hacky and I'd like a better fix.
Attachment #709742 - Flags: feedback?(wjohnston)
Using these patches, I can start a chromeless WebApp activity with a URL using:
adb shell am start -a org.mozilla.gecko.WEBAPP -n org.mozilla.fennec_mfinkle/.WebApp -d http://www.mozilla.org

A test build can be found here:
http://people.mozilla.com/~mfinkle/fennec/webapps/
Attachment #709737 - Flags: review?(bnicholson) → review+
Attachment #709738 - Flags: review?(bnicholson) → review+
Talking to mfinkle about this a bit, I think maybe a better solution is to fix bug 790457, which will allow installing a webapp via an intent. We can have two "versions" of that.

1.) You call the intent and pass in a manifest url. Fennec prompts the user, then installs the webapp like normal. We'll likely need to force a launch of the webapp when the install is done as well.

2.) You pass in the launchpath of the webapp. Fennec generates a stub manifest (we'll have to load the page once and dig around in it for some nice icons/metadata?), prompts the user, and installs it for you. Again, we launch the app when we're done.

This bug can be for the second implementation. The first we can do in bug 790457.

Thoughts?
Depends on: 790457
Attachment #709737 - Flags: checkin+
Attachment #709738 - Flags: checkin+
This patch builds on the previous one. It adds support for redirecting an incoming WEBAPP (no suffix #) intent and generating a WEBAPP# intent from it.

* Handle incoming intents in onCreate and onNewIntent paths (it would be nice to have both paths use the same code)
* If we need to redirect, register an index slot with WebAppAllocator, generate and start a WEBAPP# activity, and finish the WEBAPP activity
* The WebApp redirector activity does not need to be shown in the recent apps list

This patch starts multiple "URL as App" in their own process and with their own profile. It does burn through WEBAPP# slots faster than the Open Apps approach. We talked about making a "index slot recycler" which would release slots (and profiles).
Attachment #709742 - Attachment is obsolete: true
Attachment #709742 - Flags: feedback?(wjohnston)
Attachment #711307 - Flags: feedback?(wjohnston)
This is a simple minded patch that tracks whether we should recycle an index slot. If so, we release the slot in onDestroy, if the app was closing down.

This patch works but has me a bit worried. Only one process should access a Gecko profile. I am worried that we could break that rule:

* Open a URL as App (call it "A"): It becomes WEBAPP0 and uses profile "webapp0"
* Close the app: The index and profile are released for another app to use
* Open a different URL as App (call it "B"): The "0" slot is available so the app gets WEBAPP0 and "webapp0".
* Pick the "A" app from the "Recent Apps List": Even though the app is no longer running, Android remembers the intent used to start it. The intent used WEBAPP0 which collides with app "B".

We might be able to work around this by hiding all webapps from the "Recent Apps List" and we have considered doing that anyway (bug 837393)
Attachment #711320 - Flags: feedback?(wjohnston)
Also, I commented out the part that removes the profile when recycling because I need to test if removing before Gecko is killed will even work.
Comment on attachment 711307 [details] [diff] [review]
patch 3: open URL as na app using WebApp system

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

This seems great to me

::: mobile/android/base/GeckoApp.java
@@ +1993,5 @@
> +            Log.i(LOGTAG, "WEBAPP URL: intent - " + uri);
> +            int index = WebAppAllocator.getInstance(GeckoApp.mAppContext).findAndAllocateIndex(uri, "App", (Bitmap) null);
> +            Intent appIntent = GeckoAppShell.getWebAppIntent(index, uri);
> +            startActivity(appIntent);
> +            finish();

Do we need to call "finish" in this case? I guess this happens when you launch "WebApp" while "WebApp" is already running?
Attachment #711307 - Flags: feedback?(wjohnston) → feedback+
Comment on attachment 711320 [details] [diff] [review]
patch 4: recycle index slots for URLs

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

Yeah. While this is super simple, I think for our first run at this we're better to bite the bullet and do something nice. You're right, killing this onClose is likely going to cause some strange issues...

We talked today and could make the WebAppAllocator use a content provider instead (maybe we can have one that handles this and also reads from the webapps registry?), and start storing some info about lastLaunchTime, installMethod, etc for apps. Then we can do a query for something like "Find me the first slot that's empty, or if there are none the first non-installed app that hasn't been used in awhile". Maybe that's overkill for proof of concept work though... We could just kill the recent apps stuff and kill off that problem now.
Attachment #711320 - Flags: feedback?(wjohnston)
(In reply to Wesley Johnston (:wesj) from comment #17)
> Comment on attachment 711307 [details] [diff] [review]
> patch 3: open URL as na app using WebApp system
> 
> Review of attachment 711307 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems great to me
> 
> ::: mobile/android/base/GeckoApp.java
> @@ +1993,5 @@
> > +            Log.i(LOGTAG, "WEBAPP URL: intent - " + uri);
> > +            int index = WebAppAllocator.getInstance(GeckoApp.mAppContext).findAndAllocateIndex(uri, "App", (Bitmap) null);
> > +            Intent appIntent = GeckoAppShell.getWebAppIntent(index, uri);
> > +            startActivity(appIntent);
> > +            finish();
> 
> Do we need to call "finish" in this case? I guess this happens when you
> launch "WebApp" while "WebApp" is already running?

Actually, we don't need this. We should only handle WEBAPP (URL) intents in onCreate
This patch should be ready for review now.
* I removed the GeckoApp.onNewIntent part since we only need to handle onCreate
* I removed the extra logging

Tested on a few phones. Seems to work OK. It's possible to open more than one URL as App. The apps open in separate process and use separate profiles.
Attachment #711307 - Attachment is obsolete: true
Attachment #712189 - Flags: review?(wjohnston)
This simple APK fires the URL intent in it's onResume. This means it shows a webapp when launched and anytime it is resumed.

I have hard coded the https://mobile.twitter.com URL, so this is a "twitter" webapp APK. There is a lot of things to improve. There are also some downsides, mainly that the thumbnail used in the Recent Apps List needs to be "fixed" somehow. It currently shows whatever is behind the app (it's transparent).

Yes, you need a Firefox build with these patches in order for this APK to work.
An updated Firefox build, with the latest patches is here:
http://people.mozilla.com/~mfinkle/fennec/webapps/
Comment on attachment 712189 [details] [diff] [review]
patch 3: open URL as an app using WebAppAllocator

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

::: mobile/android/base/GeckoThread.java
@@ +103,5 @@
>          String type = getTypeFromAction(mIntent.getAction());
>          mIntent = null;
>  
>          // and then fire us up
> +        Log.i(LOGTAG, "RunGecko - args = " + args + ", type = " + type);

We probably should be careful about printing this (potentially secure) info.

::: mobile/android/base/WebApp.java.in
@@ +93,5 @@
> +    protected void initializeChrome(String uri, boolean isExternalURL) {
> +        String action = getIntent().getAction();
> +        if (GeckoApp.ACTION_WEBAPP_PREFIX.equals(action)) {
> +            int index = WebAppAllocator.getInstance(this).findAndAllocateIndex(uri, "App", (Bitmap) null);
> +            Intent appIntent = GeckoAppShell.getWebAppIntent(index, uri);

Lets add a comment here explaining that, if an app with the uri isn't installed, the allocator will install it in Java, but not in Gecko.
Attachment #712189 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e9f268fe695

We can open a new bug for the "Recycle index slots" patch
Whiteboard: [leave-open]
https://hg.mozilla.org/mozilla-central/rev/1e9f268fe695
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
I updated mfinkle's old wrapper and stated a github repo to play with it in:

https://github.com/wesj/wrapapp
You need to log in before you can comment on or make changes to this bug.