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

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Margaret, Assigned: bdahl)

Tracking

(Depends on 1 bug, Blocks 1 bug)

Trunk
Firefox 47
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed, relnote-firefox 47+, fennec47+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

4 years ago
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
(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!
Reporter

Comment 2

4 years ago
(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.

Updated

4 years ago
Blocks: 1238129

Updated

4 years ago
Depends on: 1238474
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.
No longer depends on: 1238474

Comment 4

4 years ago
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?
(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

4 years ago
> > 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.
Depends on: 1239367
Depends on: 1242787
Depends on: 1238576
Blocks: fatfennec
OS: Unspecified → Android
Hardware: Unspecified → All
Version: 35 Branch → Trunk
Summary: Remove support for WebRT → disable and remove support for Android web runtime (a.k.a. WebRT)
No longer blocks: 1245199
Assignee

Updated

3 years ago
Assignee: nobody → bdahl
Should this be tracking some release?
tracking-fennec: --- → ?
Reporter

Comment 8

3 years ago
(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) 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.
Assignee

Comment 9

3 years ago
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.
tracking-fennec: ? → 47+
Assignee

Comment 10

3 years ago
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
Attachment #8722290 - Flags: review?(margaret.leibovic)
Reporter

Comment 11

3 years ago
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.
Attachment #8722290 - Flags: review?(myk)
Attachment #8722290 - Flags: review?(margaret.leibovic)
Attachment #8722290 - Flags: feedback+
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

3 years ago
(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.
Reporter

Comment 14

3 years ago
(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

3 years ago
> 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.
(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.
(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.
Reporter

Comment 18

3 years ago
(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 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.
Attachment #8722290 - Flags: review?(myk)
Attachment #8722290 - Flags: review-
Attachment #8722290 - Flags: feedback?(fabrice)
Assignee

Updated

3 years ago
Blocks: 1251093
Assignee

Updated

3 years ago
Blocks: 1251095
(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

3 years ago
(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.
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)]:
relnote-b2g: --- → ?
(In reply to Barbara Bermes [:barbara] from comment #22)

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

see also https://wiki.mozilla.org/Marketplace/FutureofMarketplaceFAQ
Release Note Request (optional, but appreciated)
[Why is this notable]:
[Suggested wording]:
[Links (documentation, blog post, etc)]:
relnote-firefox: --- → ?
relnote-b2g: ? → ---
Assignee

Comment 25

3 years ago
Attachment #8722290 - Attachment is obsolete: true
Attachment #8722290 - Flags: feedback?(fabrice)
Attachment #8724833 - Flags: review?(myk)
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.
Attachment #8724833 - Flags: review?(myk) → review+
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 29

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/93156962855d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
Added to Fx47 (Aurora) release notes.
Reporter

Updated

3 years ago
Depends on: 1256415
You need to log in before you can comment on or make changes to this bug.