1314445 - Remove GMPService::GetPluginVersionForAPI

Status: RESOLVED FIXED

(Core :: Audio/Video: GMP, defect, P3)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

We no longer need the callers of GMPService::GetPluginVersionForAPI and the code itself.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec14d64d86a8

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7dd851c2771

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Don't pass CDM version to MediaKey*.

We don't need this, and we don't seem to be using this anyway.

Review commit: https://reviewboard.mozilla.org/r/89968/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89968/

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Make MediaKeySystemAccess use GMPService::HasPluginForAPI.

Removes callers of GMPService::GetPluginVersionForAPI().

Review commit: https://reviewboard.mozilla.org/r/89970/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89970/

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Remove GMPService::GetPluginVersionForAPI.

Review commit: https://reviewboard.mozilla.org/r/89972/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89972/

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Remove unused ParseKeySystem().

Review commit: https://reviewboard.mozilla.org/r/89974/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89974/

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Remove unused enum values from MediaKeySystemStatus.

This WebIDL enum/dict is used to pass messages from Gecko to the front-end
chrome JS code to notify it of problems starting up EME CDMs. This patch
removes some statuses from the enum that are no longer dispatched as of an
earlier patch in this series.

Review commit: https://reviewboard.mozilla.org/r/89976/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89976/

Comment 8

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Remove unused EME notification handlers from browser-media.js.

Gecko no longer emits these notifications, so there's no point handling them in
the front end code.

Review commit: https://reviewboard.mozilla.org/r/89978/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89978/

Comment 9

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Remove unused EME notification handlers from GMPProvider.jsm.

Gecko no longer emits this notification, so there's no point handling it in
GMPProvider.

Review commit: https://reviewboard.mozilla.org/r/89980/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89980/

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Remove tests for CDM versions specified in keysystem string.

Review commit: https://reviewboard.mozilla.org/r/89982/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89982/

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1314445 - Update tests for CDM request notifications to reflect new behaviour.

Review commit: https://reviewboard.mozilla.org/r/89984/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89984/

Comment 12

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8806577 [details]
Bug 1314445 - Don't pass CDM version to MediaKey*.

https://reviewboard.mozilla.org/r/89968/#review89488

Comment 13

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8806578 [details]
Bug 1314445 - Make MediaKeySystemAccess use GMPService::HasPluginForAPI.

https://reviewboard.mozilla.org/r/89970/#review89492

Comment 14

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8806579 [details]
Bug 1314445 - Remove GMPService::GetPluginVersionForAPI.

https://reviewboard.mozilla.org/r/89972/#review89494

Comment 15

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8806580 [details]
Bug 1314445 - Remove unused ParseKeySystem().

https://reviewboard.mozilla.org/r/89974/#review89496

Comment 16

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8806584 [details]
Bug 1314445 - Remove tests for CDM versions specified in keysystem string.

https://reviewboard.mozilla.org/r/89982/#review89498

Comment 17

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8806585 [details]
Bug 1314445 - Update tests for CDM request notifications to reflect new behaviour.

https://reviewboard.mozilla.org/r/89984/#review89500

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8806581 [details]
Bug 1314445 - Remove unused enum values from MediaKeySystemStatus.

https://reviewboard.mozilla.org/r/89976/#review89620

::: dom/webidl/MediaKeysRequestStatus.webidl:6
(Diff revision 1)
>  /* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this file,
>   * You can obtain one at http://mozilla.org/MPL/2.0/.
>   */
>  

Would be nice if somewhere in this file there was a comment that this isn't exposed to the web, but is gecko internal thing only.

Comment 19

[:Gijs (he/him)]

Comment on attachment 8806582 [details]
Bug 1314445 - Remove unused EME notification handlers from browser-media.js.

https://reviewboard.mozilla.org/r/89978/#review89638

::: browser/base/content/browser-media.js:94
(Diff revision 1)
>        case "cdm-not-supported":
>          // Not to pop up user-level notification because they cannot do anything
>          // about it.

This looks like it should be an early return? I realize this isn't something introduced by your patch, but we might as well fix it while we're here.

::: browser/base/content/browser-media.js:98
(Diff revision 1)
>          // Not to pop up user-level notification because they cannot do anything
>          // about it.
> -      case "error":
> -        // Fall through and do the same for unknown messages:
>        default:
> -        let typeOfIssue = status == "error" ? "error" : "message ('" + status + "')";
> +        Cu.reportError("Unknown message ('" + status + "') dealing with EME key request: " + data);

For bonus points, please use `new Error()` instead of passing a string to `Cu.reportError`.

::: browser/base/content/browser-media.js:144
(Diff revision 1)
>                             buttons);
>    },
>    showPopupNotificationForSuccess: function(browser, keySystem) {
>      // We're playing EME content! Remove any "we can't play because..." messages.
>      var box = gBrowser.getNotificationBox(browser);
> -    ["drmContentDisabled",
> +    ["drmContentDisabled", "drmContentCDMInstalling"].forEach(function (value) {

Personally, I'd prefer to keep this multilined so it's easier to add new items if we add different ones, and so we don't have to touch blame.

Comment 20

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806577 [details]
Bug 1314445 - Don't pass CDM version to MediaKey*.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89968/diff/1-2/

Comment 21

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806578 [details]
Bug 1314445 - Make MediaKeySystemAccess use GMPService::HasPluginForAPI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89970/diff/1-2/

Comment 22

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806579 [details]
Bug 1314445 - Remove GMPService::GetPluginVersionForAPI.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89972/diff/1-2/

Comment 23

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806580 [details]
Bug 1314445 - Remove unused ParseKeySystem().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89974/diff/1-2/

Comment 24

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806581 [details]
Bug 1314445 - Remove unused enum values from MediaKeySystemStatus.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89976/diff/1-2/

Comment 25

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806582 [details]
Bug 1314445 - Remove unused EME notification handlers from browser-media.js.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89978/diff/1-2/

Comment 26

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806583 [details]
Bug 1314445 - Remove unused EME notification handlers from GMPProvider.jsm.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89980/diff/1-2/

Comment 27

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806584 [details]
Bug 1314445 - Remove tests for CDM versions specified in keysystem string.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89982/diff/1-2/

Comment 28

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806585 [details]
Bug 1314445 - Update tests for CDM request notifications to reflect new behaviour.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89984/diff/1-2/

Comment 29

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8806583 [details]
Bug 1314445 - Remove unused EME notification handlers from GMPProvider.jsm.

https://reviewboard.mozilla.org/r/89980/#review89872

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84a6f16feabd
Don't pass CDM version to MediaKey*. r=gerald
https://hg.mozilla.org/integration/autoland/rev/7fbf411102d8
Make MediaKeySystemAccess use GMPService::HasPluginForAPI. r=gerald
https://hg.mozilla.org/integration/autoland/rev/ce3ab5d4e29d
Remove GMPService::GetPluginVersionForAPI. r=gerald
https://hg.mozilla.org/integration/autoland/rev/06b9f9ad45ab
Remove unused ParseKeySystem(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/06fd311735c7
Remove unused enum values from MediaKeySystemStatus. r=smaug
https://hg.mozilla.org/integration/autoland/rev/18558217586a
Remove unused EME notification handlers from browser-media.js. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/bd3344c860a0
Remove unused EME notification handlers from GMPProvider.jsm. r=cpearce
https://hg.mozilla.org/integration/autoland/rev/ba37b01751a2
Remove tests for CDM versions specified in keysystem string. r=gerald
https://hg.mozilla.org/integration/autoland/rev/4eb93831e7d3
Update tests for CDM request notifications to reflect new behaviour. r=gerald

Comment 31

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #29)
> Comment on attachment 8806583 [details]
> Bug 1314445 - Remove unused EME notification handlers from GMPProvider.jsm.
> 
> https://reviewboard.mozilla.org/r/89980/#review89872

This is me converting the r+ spohl applied to Attachment #8806583 [details] into a format mozreview can understand so I can autoland.

Comment 32

[Stephen A Pohl [:spohl]]

Comment on attachment 8806583 [details]
Bug 1314445 - Remove unused EME notification handlers from GMPProvider.jsm.

https://reviewboard.mozilla.org/r/89980/#review89874

Comment 33

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Fix test_eme_request_notifications to not fail on WinXP

MozReview-Commit-ID: 5zbySCZtI1b

Comment 34

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Fix test_eme_request_notifications to not fail on WinXP

MozReview-Commit-ID: 5zbySCZtI1b

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8891610c7c7b
Fix test_eme_request_notifications to not fail on WinXP. r=bustage

Comment 36

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/84a6f16feabd
https://hg.mozilla.org/mozilla-central/rev/7fbf411102d8
https://hg.mozilla.org/mozilla-central/rev/ce3ab5d4e29d
https://hg.mozilla.org/mozilla-central/rev/06b9f9ad45ab
https://hg.mozilla.org/mozilla-central/rev/06fd311735c7
https://hg.mozilla.org/mozilla-central/rev/18558217586a
https://hg.mozilla.org/mozilla-central/rev/bd3344c860a0
https://hg.mozilla.org/mozilla-central/rev/ba37b01751a2
https://hg.mozilla.org/mozilla-central/rev/4eb93831e7d3
https://hg.mozilla.org/mozilla-central/rev/8891610c7c7b