Autophone - S1S2 -Throbber stop regression on 2015-01-05

RESOLVED FIXED in Firefox 37

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bc, Assigned: mfinkle)

Tracking

(Blocks: 1 bug, {regression})

Trunk
Firefox 37
ARM
Android
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(fennec37+)

Details

(URL)

Attachments

(1 attachment)

Created attachment 8545913 [details] [diff] [review]
openh264-better-delay v0.1

When Firefox tries to do work during startup, this is what happens. Sadly, it seems we are limited by how we can install the OpenH264 add-on code, so let's try to match Desktop here. This patch:
1. Moves the install check into the dispatch queue
2. Waits one minute before calling the init (like Desktop)

See http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#1351
Assignee: nobody → mark.finkle
Attachment #8545913 - Flags: review?(margaret.leibovic)

Comment 2

3 years ago
Comment on attachment 8545913 [details] [diff] [review]
openh264-better-delay v0.1

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

::: mobile/android/chrome/content/browser.js
@@ +376,5 @@
> +          // Delay this a minute because there's no rush
> +          setTimeout(() => {
> +            BrowserApp.gmpInstallManager = new GMPInstallManager();
> +            BrowserApp.gmpInstallManager.simpleCheckAndInstall().then(null, () => {});
> +          }, 1000 * 60);

I'm not familiar with this GMPInstallManager business. How certain do we want to be that we call this code? Unlike desktop, Fennec doesn't always run for a whole minute before getting killed. If we go with this approach, we need to be okay with that.

Maybe we should use a pref to keep track of how many times we've punted on this check, and if we still haven't done it after something like 10 app runs, we should just do it sooner.
Attachment #8545913 - Flags: review?(margaret.leibovic) → review+
tracking-fennec: ? → 37+
(In reply to :Margaret Leibovic from comment #2)
> Comment on attachment 8545913 [details] [diff] [review]
> openh264-better-delay v0.1
> 
> Review of attachment 8545913 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/chrome/content/browser.js
> @@ +376,5 @@
> > +          // Delay this a minute because there's no rush
> > +          setTimeout(() => {
> > +            BrowserApp.gmpInstallManager = new GMPInstallManager();
> > +            BrowserApp.gmpInstallManager.simpleCheckAndInstall().then(null, () => {});
> > +          }, 1000 * 60);
> 
> I'm not familiar with this GMPInstallManager business. How certain do we
> want to be that we call this code? Unlike desktop, Fennec doesn't always run
> for a whole minute before getting killed. If we go with this approach, we
> need to be okay with that.

I think we are OK for now. That could change though, as this feature starts being used more often.

> Maybe we should use a pref to keep track of how many times we've punted on
> this check, and if we still haven't done it after something like 10 app
> runs, we should just do it sooner.

We were talking about ways around this issue on IRC. I'd like to remove it entirely from browser.js and put it into a background service. This is essentially and specific downloader for video plugins. If we need to add some complexity, I'd rather spend it by removing it completely from our startup path.
(Reporter)

Comment 5

3 years ago
Looks like comment 3 definitely helped with nexus s on fx-team:

http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstop/local-twitter/norejected/2015-01-08/2015-01-08/notcached/errorbars/standarderror

https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=b336e9272939&tochange=2035148cfa4d

This wasn't as dramatic a change on my local nexus one though testing before and after with custom builds...
https://hg.mozilla.org/mozilla-central/rev/8e516e029c8e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
You need to log in before you can comment on or make changes to this bug.