1235869 - disable and remove support for Android web runtime (a.k.a. WebRT)

Status: RESOLVED FIXED

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

Description

[:Margaret Leibovic]

We've been discussing this for a while, time to file a bug.

See also:

https://mozilla.aha.io/features/FENN-266
https://mail.mozilla.org/pipermail/firefox-dev/2015-December/003663.html

Comment 1

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

(In reply to :Margaret Leibovic from comment #0)
> https://mail.mozilla.org/pipermail/firefox-dev/2015-December/003663.html

Note that I haven't yet decided to disable the runtime, which I proposed in that thread, so the proposal is currently still pending.  I do expect to make the final decision soon, I'm just going to wait a bit longer to make sure that people have an opportunity to participate in the conversation, especially since it's the holidays in many parts of the world.

Assuming no one changes my mind, I'll then file a set of bugs to track the various (cross-product) dependencies involved with disabling the runtimes (on both desktop and Android).  I hope it's alright to stitch this bug into the fabric of dependencies at that time!

Comment 2

[:Margaret Leibovic]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #1)
> (In reply to :Margaret Leibovic from comment #0)
> > https://mail.mozilla.org/pipermail/firefox-dev/2015-December/003663.html
> 
> Note that I haven't yet decided to disable the runtime, which I proposed in
> that thread, so the proposal is currently still pending.  I do expect to
> make the final decision soon, I'm just going to wait a bit longer to make
> sure that people have an opportunity to participate in the conversation,
> especially since it's the holidays in many parts of the world.
> 
> Assuming no one changes my mind, I'll then file a set of bugs to track the
> various (cross-product) dependencies involved with disabling the runtimes
> (on both desktop and Android).  I hope it's alright to stitch this bug into
> the fabric of dependencies at that time!

Sounds like a great plan!

Apologies if I jumped the gun on filing a bug, I just wanted a place to track the outcome of that discussion.

Comment 3

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

SUN Haitao wrote in bug 1238474:
>Currently, there is no easy way to use iframe[mozbrowser] on Fennec and Desktop except Open Web Apps. With this mechanism is deprecated, a new solution is needed.

I understand that disabling the runtimes will prevent the use of iframe[mozbrowser], and that will be a problem for apps that use it.  But I'm not going to require that all features of the runtimes be reimplemented before they're disabled, since Firefox/Fennec have never promised that those features will be supported, and in any case there aren't engineering resources to reimplement them.  (If there were, then we wouldn't be in this situation in the first place.)

So I'm removing the dependency on bug 1238474.  I do think that's a reasonable feature request, however, and I encourage you to pursue it independently of this bug.  Also note bug 1238160, which is about enabling mozbrowser on desktop for chrome code.

Comment 4

[SUN Haitao]

Based on my personal experiences, the most important capacities that should remain are:

a) Open an app in its own Android task (Bug 1208195).
b) Open an app in 'standalone' mode (Bug 1126479?).
c) Open an app in 'minimal-ui' mode (Bug ????????).

C is not required in the first stage, but should be considered while implementing B.

All of these items requires a special activity on Android. Due to Bug 1191028, some tricks to force every such activity in its own process are required currently . Should we wait Bug 1191028 or re-implement the tricks or refactor WebappImpl which are already using such tricks?

Comment 5

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

(In reply to SUN Haitao from comment #4)
> c) Open an app in 'minimal-ui' mode (Bug ????????).

Can you create a bug for this feature request and explain the idea in the bug description?  I'm unfamiliar with it, and it'd be helpful to have more information!  Ideally in a new bug report, so we can focus this one on the work it specifies.


> All of these items requires a special activity on Android. Due to Bug
> 1191028, some tricks to force every such activity in its own process are
> required currently . Should we wait Bug 1191028 or re-implement the tricks
> or refactor WebappImpl which are already using such tricks?

I'm not sure that WebappImpl code would actually be useful for implementing those features.  If so, however, then their implementors would be welcome to reuse it.

But that still wouldn't block this work to disable the runtimes, nor do the features themselves.  The only blockers for this work are existing features in other products that depend on the presence of the Android runtime: Gecko exposure of the mozApps API on Android, Marketplace solicitation and distribution of apps for Android, MDN documentation of apps for Android, and the like.

(That being said, both the Fennec and the WADI engineering teams are interested in supporting and promoting progressive apps in Fennec, and the two bugs you referenced are both dependencies of the "Progressive Apps in Fennec" meta-bug 1212648, so they could happen in conjunction with, or even before, this work to disable the runtime.)

Comment 6

[SUN Haitao]

> > c) Open an app in 'minimal-ui' mode (Bug ????????).
I filed Bug 1239358 for this.

> I'm not sure that WebappImpl code would actually be useful for implementing
> those features.  If so, however, then their implementors would be welcome to
> reuse it.
My tinkering to make Fennec a standalone web app runtime suggests there are several recyclable workarounds to open an app in its own Android task. But if bug 1191028 lands soon enough, such work can be evicted.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Should this be tracking some release?

Comment 8

[:Margaret Leibovic]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #7)
> Should this be tracking some release?

I want this gone as soon as possible. Not sure if that's realistic for 47 or if we need to wait until 48.

Comment 9

[Brendan Dahl [:bdahl]]

47 sounds reasonable. I'm currently working on disabling/fixing more tests. Even after bug 1238576 there were a number of them that used mozapps stuff behind the scenes that need to be disabled for non b2g/mulet builds.

Comment 10

[Brendan Dahl [:bdahl]]

Attachment: 0001-Bug-1235869-Remove-web-runtime-from-android.patch

This removes the majority of the web apps runtime code. I had to disable three font tests that now permanently fail and have nothing to do with the code removal. I've filed bug 1250229 to investigate this.
There's still some code that needs to be removed, but it will require touching more stuff outside of android and I'll need to coordinate with Myk on this when he removes web runtime from desktop. The code still needed to be removed is:
  - mobile/android/components/DirectoryProvider.js (handling the WEBAPPS_DIR)
  - mobile/android/chrome/content/browser.js (DOMApplicationRegistry)
Stop including the following in mobile/android/installer/package-manifest.in:
    - @BINPATH@/components/Webapps.js
    - @BINPATH@/components/Webapps.manifest
    - @BINPATH@/components/AppsService.js
    - @BINPATH@/components/AppsService.manifest


Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b0828ff29570

Comment 11

[:Margaret Leibovic]

Comment on attachment 8722290 [details] [diff] [review]
0001-Bug-1235869-Remove-web-runtime-from-android.patch

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

This generally seems fine to me, but I'm not completely familiar with this code, so there may be things that we're missing here. I think Myk should also take a look at this.

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ -691,4 @@
>      @Override
>      public void handleMessage(String event, JSONObject message) {
>          try {
> -            if (event.equals("Gecko:DelayedStartup")) {

You could also move where we register/unregister this listener to BrowserApp, since it will only be used by BrowserApp.

@@ +2657,4 @@
>      }
>  
>      protected boolean getIsDebuggable() {
> +        // Return false so Fennec doesn't appear to be debuggable.

You can make this into a private method, since it was only designed to be overridden for web apps.

::: mobile/android/base/resources/layout/shared_ui_components.xml
@@ +4,4 @@
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <!-- This file is used to include shared UI components in different gecko app
> +     layouts, such as gecko_app.xml -->

We should file a follow-up bug to put this file directly back into gecko_app.xml, since that will be the only consumer.

Comment 12

[Eugen Sawin [:esawin]]

I think WebRT was the only consumer of the system message components on Android, which no longer needs to packaged (https://hg.mozilla.org/mozilla-central/rev/eea1f32a8e25).

Comment 13

[SUN Haitao]

(In reply to :Margaret Leibovic from comment #11)

> ::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
> @@ -691,4 @@
> >      @Override
> >      public void handleMessage(String event, JSONObject message) {
> >          try {
> > -            if (event.equals("Gecko:DelayedStartup")) {
> 
> You could also move where we register/unregister this listener to
> BrowserApp, since it will only be used by BrowserApp.

> ::: mobile/android/base/resources/layout/shared_ui_components.xml
> @@ +4,4 @@
> >     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> >  
> >  <!-- This file is used to include shared UI components in different gecko app
> > +     layouts, such as gecko_app.xml -->
> 
> We should file a follow-up bug to put this file directly back into
> gecko_app.xml, since that will be the only consumer.

We'd better do these after making sure Progressive Apps will not use them.

Comment 14

[:Margaret Leibovic]

(In reply to SUN Haitao from comment #13)
> (In reply to :Margaret Leibovic from comment #11)
> 
> > ::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
> > @@ -691,4 @@
> > >      @Override
> > >      public void handleMessage(String event, JSONObject message) {
> > >          try {
> > > -            if (event.equals("Gecko:DelayedStartup")) {
> > 
> > You could also move where we register/unregister this listener to
> > BrowserApp, since it will only be used by BrowserApp.
> 
> > ::: mobile/android/base/resources/layout/shared_ui_components.xml
> > @@ +4,4 @@
> > >     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> > >  
> > >  <!-- This file is used to include shared UI components in different gecko app
> > > +     layouts, such as gecko_app.xml -->
> > 
> > We should file a follow-up bug to put this file directly back into
> > gecko_app.xml, since that will be the only consumer.
> 
> We'd better do these after making sure Progressive Apps will not use them.

This is only used by WebApp.java, which is unused now, so this isn't an issue.

Progressive web apps are launched as normal tabs, and they aren't using any of this old web apps logic.

Comment 15

[SUN Haitao]

> This is only used by WebApp.java, which is unused now, so this isn't an
> issue.
> 
> Progressive web apps are launched as normal tabs, and they aren't using any
> of this old web apps logic.

According to http://w3c.github.io/manifest/#display-modes, there are four display modes can be used. Two of them ('standalone' and 'minimal-ui') do not look like normal tabs. Of course, it is possible to implement them as normal tabs in special states. But implementing as special activities may be easier to maintain.

Comment 16

[Marco Castelluccio [:marco]]

(In reply to SUN Haitao from comment #15)
> > This is only used by WebApp.java, which is unused now, so this isn't an
> > issue.
> > 
> > Progressive web apps are launched as normal tabs, and they aren't using any
> > of this old web apps logic.
> 
> According to http://w3c.github.io/manifest/#display-modes, there are four
> display modes can be used. Two of them ('standalone' and 'minimal-ui') do
> not look like normal tabs. Of course, it is possible to implement them as
> normal tabs in special states. But implementing as special activities may be
> easier to maintain.

You might want to follow bug 1208195 and bug 1212648.

Comment 17

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

(In reply to SUN Haitao from comment #15)
> > This is only used by WebApp.java, which is unused now, so this isn't an
> > issue.
> > 
> > Progressive web apps are launched as normal tabs, and they aren't using any
> > of this old web apps logic.
> 
> According to http://w3c.github.io/manifest/#display-modes, there are four
> display modes can be used. Two of them ('standalone' and 'minimal-ui') do
> not look like normal tabs. Of course, it is possible to implement them as
> normal tabs in special states. But implementing as special activities may be
> easier to maintain.

Instead of leaving this code in the tree just in case it'll be useful, I would redevelop the logic as needed when we implement support for these display modes.  After all, we don't know that we'll be able to reuse the code, nor is it clear that reusing it will be faster than reimplementing it.  And the code will remain in history, if we want to consult it at some point.

Comment 18

[:Margaret Leibovic]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #17)
> (In reply to SUN Haitao from comment #15)
> > > This is only used by WebApp.java, which is unused now, so this isn't an
> > > issue.
> > > 
> > > Progressive web apps are launched as normal tabs, and they aren't using any
> > > of this old web apps logic.
> > 
> > According to http://w3c.github.io/manifest/#display-modes, there are four
> > display modes can be used. Two of them ('standalone' and 'minimal-ui') do
> > not look like normal tabs. Of course, it is possible to implement them as
> > normal tabs in special states. But implementing as special activities may be
> > easier to maintain.
> 
> Instead of leaving this code in the tree just in case it'll be useful, I
> would redevelop the logic as needed when we implement support for these
> display modes.  After all, we don't know that we'll be able to reuse the
> code, nor is it clear that reusing it will be faster than reimplementing it.
> And the code will remain in history, if we want to consult it at some point.

+1

Comment 19

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

Comment on attachment 8722290 [details] [diff] [review]
0001-Bug-1235869-Remove-web-runtime-from-android.patch

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

Overall, this looks great.  Just a few minor issues…

There are a couple of references to webapp_generic_name, which is no longer being used (was used only by WebappManifestFragment.xml.frag.in).

There's also a webapp_titlebar_bg.xml file, which I think is no longer being used.  It's only used by WebView.Titlebar (in mobile/android/base/resources/values/styles.xml), which itself is not being used, as far as I can tell.  This was added in bug 888145, and it is related to the runtime, but it might be worth removing it in a followup in case there's a hidden dependency, as I'm not familiar with this corner of the codebase.

::: mobile/android/app/mobile.js
@@ -876,5 @@
>  pref("browser.snippets.syncPromo.enabled", true);
>  pref("browser.snippets.firstrunHomepage.enabled", true);
>  
> -// The URL of the APK factory from which we obtain APKs for webapps.
> -pref("browser.webapps.apkFactoryUrl", "https://controller.apk.firefox.com/application.apk");

These prefs are also defined in mobile/android/b2gdroid/app/b2gdroid.js.  I doubt they're being used, but requesting feedback from fabrice in case they are, so he knows about these changes.

::: mobile/android/base/java/org/mozilla/gecko/GeckoApp.java
@@ +2657,4 @@
>      }
>  
>      protected boolean getIsDebuggable() {
> +        // Return false so Fennec doesn't appear to be debuggable.

Indeed, you can remove this entirely (and the "NativeApp:IsDebuggable" handler), since it no longer ever gets overridden (nor is the event ever triggered anymore).

::: mobile/android/base/java/org/mozilla/gecko/GeckoProfile.java
@@ +1030,4 @@
>              Log.w(LOGTAG, "Couldn't write times.json.", e);
>          }
>  
> +        // Initialize pref flag for displaying the start pane for a new profiles.

Nit: a new profiles -> "new profiles" or "a new profile"

::: mobile/android/chrome/content/browser.js
@@ -4355,4 @@
>    onLocationChange: function(aWebProgress, aRequest, aLocationURI, aFlags) {
>      let contentWin = aWebProgress.DOMWindow;
>  
> -    // Browser webapps may load content inside iframes that can not reach across the app/frame boundary

This code is actually unrelated to the runtime.  It comes from bug 888145 and was discovered by wesj when he ran Gaia inside Fennec proper (i.e. not using a synthetic APK).

It's possible that the code is obsolete, for different reasons, but we should remove it separately if so rather than as part of this changeset.

::: mobile/android/modules/WebappManager.jsm
@@ -1,1 @@
> -/* This Source Code Form is subject to the terms of the Mozilla Public

There's a reference to this file in .eslintignore, and another one in mobile/android/.eslintrc.  Those should also be removed.

Comment 20

[[:fabrice] Fabrice Desré]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #19)

> > -// The URL of the APK factory from which we obtain APKs for webapps.
> > -pref("browser.webapps.apkFactoryUrl", "https://controller.apk.firefox.com/application.apk");
> 
> These prefs are also defined in mobile/android/b2gdroid/app/b2gdroid.js.  I
> doubt they're being used, but requesting feedback from fabrice in case they
> are, so he knows about these changes.

It's safe to remove them from b2gdroid.js too. Thanks for catching that!

Comment 21

[SUN Haitao]

(In reply to Marco Castelluccio [:marco] from comment #16)
> You might want to follow bug 1208195 and bug 1212648.

I am following bug 1208195 and several other related bugs of bug 1212648.

(In reply to Myk Melez [:myk] [@mykmelez] from comment #17)
> display modes.  After all, we don't know that we'll be able to reuse the
> code, nor is it clear that reusing it will be faster than reimplementing it.
> And the code will remain in history, if we want to consult it at some point.

I believe this is a nice plan with good reason.

Comment 22

[Barbara Bermes [:barbara] - NI please!]

Release Note Request (optional, but appreciated)
[Why is this notable]: Removing references and links to WebRT
[Suggested wording]: Remove support for Android web runtime (WebRT)
[Links (documentation, blog post, etc)]:

Comment 23

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

(In reply to Barbara Bermes [:barbara] from comment #22)

> [Links (documentation, blog post, etc)]:

see also https://wiki.mozilla.org/Marketplace/FutureofMarketplaceFAQ

Comment 24

[Barbara Bermes [:barbara] - NI please!]

Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:

Comment 25

[Brendan Dahl [:bdahl]]

Attachment: 0001-Bug-1235869-Remove-web-runtime-from-android.patch

Comment 26

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

Comment on attachment 8724833 [details] [diff] [review]
0001-Bug-1235869-Remove-web-runtime-from-android.patch

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

Looks great!

I filed followup bug 1252276 to remove the runtime-related prefs from b2gdroid.js.

Comment 27

[Brendan Dahl [:bdahl]]

^ try run https://treeherder.mozilla.org/#/jobs?repo=try&revision=58650aa36c50

Comment 28

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/93156962855d

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/93156962855d

Comment 30

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Added to Fx47 (Aurora) release notes.