Widevine installation failure

RESOLVED FIXED in Firefox 52

Status

()

Core
Audio/Video: Playback
P1
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: kentuckyfriedtakahe, Assigned: daleharvey)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52+ verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Widevine is failing to install with the following error. I'm assuming it is a regression in caused by bug 1267495.

14:16:25.357 1475198185357	Toolkit.GMP	ERROR	GMPWrapper(gmp-widevinecdm) findUpdates() - updateTask for gmp-widevinecdm threw: TypeError: gmpAddons.find is not a function (resource://gre/modules/addons/GMPProvider.jsm:295:22) JS Stack trace: GMPWrapper.prototype.findUpdates/this._updateTask<@GMPProvider.jsm:295:22 1 Log.jsm:753
	App_append resource://gre/modules/Log.jsm:753:9
	Logger.prototype.log resource://gre/modules/Log.jsm:389:7
	LoggerRepository.prototype.getLoggerWithMessagePrefix/proxy.log resource://gre/modules/Log.jsm:504:44
	Logger.prototype.error resource://gre/modules/Log.jsm:397:5
	GMPWrapper.prototype.findUpdates/this._updateTask< resource://gre/modules/addons/GMPProvider.jsm:306:9
	next self-hosted
	TaskImpl_run resource://gre/modules/Task.jsm:322:42
	bound TaskImpl_run self-hosted
	Handler.prototype.process resource://gre/modules/Promise-backend.js:937:23
	this.PromiseWalker.walkerLoop resource://gre/modules/Promise-backend.js:816:7
	bound  self-hosted
	bound bound  self-hosted
Assignee: nobody → dale
(Assignee)

Comment 1

2 years ago
Yup definitely caused by 1267495. I am confused though, does anyone more familiar with this code know why there are 3 different code paths to installing these addons? at the least the XPIProvider and browser.js path get called at some point on startup

http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#3086
http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/GMPProvider.jsm#294
http://searchfox.org/mozilla-central/source/browser/base/content/browser.js#1334

Also Anthony could you say what you do to hit this? when I open up a fresh profile I got via the browser.js method successfully
Flags: needinfo?(ajones)
(In reply to Dale Harvey (:daleharvey) from comment #1)
> Yup definitely caused by 1267495. I am confused though, does anyone more
> familiar with this code know why there are 3 different code paths to
> installing these addons? at the least the XPIProvider and browser.js path
> get called at some point on startup
> 

Maybe rhelmer knows?
Flags: needinfo?(rhelmer)
(In reply to Dale Harvey (:daleharvey) from comment #1)
> Yup definitely caused by 1267495. I am confused though, does anyone more
> familiar with this code know why there are 3 different code paths to
> installing these addons? at the least the XPIProvider and browser.js path
> get called at some point on startup
> 
> http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/XPIProvider.jsm#3086

This is a system add-on check (these are based on the way GMP works but should not be directly related otherwise).

> http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/GMPProvider.jsm#294

This is the periodic (timer-based) check for GMP.

> http://searchfox.org/mozilla-central/source/browser/base/content/browser.
> js#1334

This one I am less familiar with, but I assume this is so we don't wait for the once-every-24h-timer in case the user doesn't have any GMP addons installed, so it doesn't block startup but runs shortly after first-run so DRM videos work?
Flags: needinfo?(rhelmer)
(In reply to Dale Harvey (:daleharvey) from comment #1)
> Yup definitely caused by 1267495. I am confused though, does anyone more
> familiar with this code know why there are 3 different code paths to
> installing these addons? at the least the XPIProvider and browser.js path
> get called at some point on startup
> 
> http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/XPIProvider.jsm#3086
> http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/
> internal/GMPProvider.jsm#294
> http://searchfox.org/mozilla-central/source/browser/base/content/browser.
> js#1334
> 
> Also Anthony could you say what you do to hit this? when I open up a fresh
> profile I got via the browser.js method successfully

From the line of code this is failing on, I bet this is only happening via the periodic timer check not the on-first-run check, and that `installManager.checkForAddons()` is returning something that the next line does not expect:

https://dxr.mozilla.org/mozilla-central/rev/9baec74b3db1bf005c66ae2f50bafbdb02c3be38/toolkit/mozapps/extensions/internal/GMPProvider.jsm#294-295

The return type was changed in bug 1267495 so that would make sense, line 295 probably needs to use the .gmpAddons property:

https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#95-98

You can force a timer check using https://addons.mozilla.org/en-US/firefox/addon/timer-fire (or by pasting the right incantation into the console but I test this sort of thing enough I just keep Mossop's addon installed :))
Flags: needinfo?(dale)
(In reply to Robert Helmer [:rhelmer] from comment #3)
> (In reply to Dale Harvey (:daleharvey) from comment #1)
> > Yup definitely caused by 1267495. I am confused though, does anyone more
> > familiar with this code know why there are 3 different code paths to
> > installing these addons? at the least the XPIProvider and browser.js path
> > get called at some point on startup
> > 
> > http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/
> > internal/XPIProvider.jsm#3086

Hm ok this is a bit confusing/misleading - both the system add-on and GMP check are initiated here. We should consider separating this for clarity.

In any case - I am having trouble reproducing this too. I wonder if it only happens when updates are actually available, and only by the periodic and not the startup check?
I don't know it happened but the GMPs all became uninstalled but the others reinstalled themselves. The error I posted was as a result of playing a video on Netflix which should trigger an install.
Flags: needinfo?(ajones)
Priority: -- → P1
(Assignee)

Comment 7

2 years ago
Created attachment 8797573 [details] [diff] [review]
1306516-1.patch

So if I comment out the timer checks @ http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/GMPProvider.jsm#264,287 then use the "Fire Timer" addon and trigger an addon update check then I can reproduce this.

The fix is fairly obvious but I want to be able to reproduce to verify
Flags: needinfo?(dale)
Attachment #8797573 - Flags: review?(mconley)
(Assignee)

Comment 8

2 years ago
Comment on attachment 8797573 [details] [diff] [review]
1306516-1.patch

Clearing review and gonna see if I can form a failing test out of http://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/test/browser/browser_gmpProvider.js before landing
Attachment #8797573 - Flags: review?(mconley)
(Assignee)

Comment 9

2 years ago
Created attachment 8798413 [details] [diff] [review]
Fix expected return value from checkForAddons.

These tests actually break with the fixed code, so there are tests against conformance to the implementation but require the mock to be kept up to date.
Attachment #8797573 - Attachment is obsolete: true
Attachment #8798413 - Flags: review?(mconley)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1307997
Comment on attachment 8798413 [details] [diff] [review]
Fix expected return value from checkForAddons.

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

This looks fine to me, though I note that there's another case where we need to update another install manager mock (see comment below).

Before landing this, can you do a quick sweep through the code for all uses of checkForAddons to make sure they're all conformant to the new API? If so, with the test_gmpProvider.js test fixed, and the tests passing, r=me.

::: toolkit/mozapps/extensions/test/browser/browser_gmpProvider.js
@@ +36,5 @@
>  function MockGMPInstallManager() {
>  }
>  
>  MockGMPInstallManager.prototype = {
> +  checkForAddons: () => Promise.resolve({

Looks like there's another one of these here:

http://searchfox.org/mozilla-central/rev/76609a05d6ef7ba4223ed79e479c73fb2543a107/toolkit/mozapps/extensions/test/xpcshell/test_gmpProvider.js#45
Attachment #8798413 - Flags: review?(mconley) → review+
Tracking 52+ since it is important due to some users not being able to play Netflix.
tracking-firefox52: --- → +

Comment 14

2 years ago
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/7ff5c0b99e19
Fix expected return value from checkForAddons. r=mconley

Comment 15

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7ff5c0b99e19
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
I noticed that Widevine does not automatically install on Windows 64-bit latest Nightly 52.0a1 build (I waited even more then two hours), and based on bug 1307997 this should have fixed this. 
Note that if I hit Check for Updates, Widevine will install successfully. I've tested on Windows 10, 7, 8.1 using latest Nightly. On Mac I did not encounter this. 
Should we reopen this issue or log a new one?
Flags: needinfo?(dale)
(Assignee)

Comment 17

2 years ago
This has been filed as a new bug @ https://bugzilla.mozilla.org/show_bug.cgi?id=1309463, thanks for the extra information to debug
Flags: needinfo?(dale)
This is verified fixed on 52 per Comment 16 and Comment 17.
status-firefox52: fixed → verified
Depends on: 1309463
You need to log in before you can comment on or make changes to this bug.