GMPProvider busted on Android

RESOLVED FIXED in Firefox 39

Status

()

Core
Audio/Video
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: snorp, Assigned: snorp)

Tracking

({regression})

unspecified
mozilla41
All
Android
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 unaffected, firefox38.0.5 unaffected, firefox39 fixed, firefox40+ fixed, firefox41+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Not sure when this started, but with a local build I see the following in logcat when trying to use H264 with WebRTC:

Exception calling provider GMPProvider.startup: TypeError: this._info is undefined (resource://gre/modules/addons/GMPProvider.jsm:483:6) JS Stack trace: GMPWrapper.prototype._arePluginFilesOnDisk@GMPProvider.jsm:483:7 < GMPWrapper.prototype.validate@GMPProvider.jsm:493:12 < GMPProvider.startup@GMPProvider.jsm:520:14 < callProvider@AddonManager.jsm:208:12 < _startProvider@AddonManager.jsm:670:5 < AMI_startup@AddonManager.jsm:838:9 < AMP_startup@AddonManager.jsm:2508:5 < AMC_observe@addonManager.js:55:7" {file: "resource://gre/modules/Log.jsm" line: 749}]
This was caused by the patch in bug 1156566
Blocks: 1156566
Created attachment 8608825 [details] [diff] [review]
Fix GMPProvider on Android
Attachment #8608825 - Flags: review?(cpearce)
Comment on attachment 8608825 [details] [diff] [review]
Fix GMPProvider on Android

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

That's much nicer! :)

I uplifted the regressor, so we'll need to do the same if GMPs are supposed to work on Android on 39 or 40.

::: toolkit/mozapps/extensions/internal/GMPProvider.jsm
@@ -463,5 @@
>        f.initWithPath(path);
>        return f.exists();
>      };
>  
> -    // Determine the name of the GMP dynamic library; it differs on every

I have basically the same code in test_gmpProvider.js in createMockPluginFilesIfNeeded(), can we do the same thing there? Does that test run on Android?
Attachment #8608825 - Flags: review?(cpearce) → review+
Created attachment 8609392 [details] [diff] [review]
Fix GMPProvider on Android
Attachment #8608825 - Attachment is obsolete: true
Attachment #8609392 - Flags: review+
(In reply to Chris Pearce (:cpearce) from comment #3)
> Comment on attachment 8608825 [details] [diff] [review]
> Fix GMPProvider on Android
> 
> Review of attachment 8608825 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> That's much nicer! :)
> 
> I uplifted the regressor, so we'll need to do the same if GMPs are supposed
> to work on Android on 39 or 40.

OK.

> 
> ::: toolkit/mozapps/extensions/internal/GMPProvider.jsm
> @@ -463,5 @@
> >        f.initWithPath(path);
> >        return f.exists();
> >      };
> >  
> > -    // Determine the name of the GMP dynamic library; it differs on every
> 
> I have basically the same code in test_gmpProvider.js in
> createMockPluginFilesIfNeeded(), can we do the same thing there? Does that
> test run on Android?

This apparently does not run on Android (not totally sure why), but I fixed the test up too anyway.

Comment 6

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e140d3141a8
https://hg.mozilla.org/mozilla-central/rev/9e140d3141a8
Assignee: nobody → snorp
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Blocks: 1170540
Comment on attachment 8609392 [details] [diff] [review]
Fix GMPProvider on Android

Approval Request Comment
[Feature/regressing bug #]: Bug 1156566l, EME
[User impact if declined]: Bug 1156566 introduced a regression on Linux; the OpenH264 plugin on some Linux systems will be installed correctly, but deleted when the browser restarts and then re-downloaded 60 seconds later. Without this path uplifted, that behavior will happen.
[Describe test coverage new/current, TreeHerder]: This has unit tests, though not to cover the browser restart scenario which is causing problems.
[Risks and why]: Pretty low, just changes how we determine the file extension for dynamic libraries.
[String/UUID change made/needed]: None.
Attachment #8609392 - Flags: approval-mozilla-beta?
Attachment #8609392 - Flags: approval-mozilla-aurora?
I must not forget to ensure this gets uplifted.
Flags: needinfo?(cpearce)
status-firefox38: --- → unaffected
status-firefox38.0.5: --- → unaffected
status-firefox39: --- → affected
status-firefox40: --- → affected
tracking-firefox40: --- → ?
tracking-firefox41: --- → ?
Keywords: regression
Comment on attachment 8609392 [details] [diff] [review]
Fix GMPProvider on Android

Approved for uplift to beta and aurora, so that we don't make Linux users download OpenH264 repeatedly.
Attachment #8609392 - Flags: approval-mozilla-beta?
Attachment #8609392 - Flags: approval-mozilla-beta+
Attachment #8609392 - Flags: approval-mozilla-aurora?
Attachment #8609392 - Flags: approval-mozilla-aurora+
Chris, want to file a bug for some future test-writing to cover this scenario?
(In reply to Liz Henry (:lizzard) from comment #11)
> Chris, want to file a bug for some future test-writing to cover this
> scenario?

bug 1171300.
https://hg.mozilla.org/releases/mozilla-aurora/rev/6338c30d8da3
status-firefox40: affected → fixed
https://hg.mozilla.org/releases/mozilla-beta/rev/e2ed03987d19
status-firefox39: affected → fixed
Flags: needinfo?(cpearce)
This was approved for uplift to Aurora already. Adding a tracking flag for FF40 and FF41 as this is a regression that is painful from an end user point of view.
tracking-firefox40: ? → +
tracking-firefox41: ? → +
You need to log in before you can comment on or make changes to this bug.