Closed Bug 1134399 Opened 5 years ago Closed 5 years ago

[e10s] Using context-menu media controls in remote browser causes unsafe CPOW usage warning

Categories

(Firefox :: Menus, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 38
Tracking Status
e10s m8+ ---
firefox38 --- fixed

People

(Reporter: Kwan, Assigned: Kwan)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1133577 +++

STR:

1) Visit a site with a <video/> or <audio/> on it in an e10s window
2) Right-click on the media, and choose one of the media control commands [Play, Pause, Mute, Unmute, Play Speed, Hide Controls, Show Controls, Hide Statistics, Show Statistics]

This causes an "unsafe CPOW usage" warning in the Browser Console (or three for the statistics commands).

In browser/base/content/nsContextMenu.js:

  mediaCommand : function CM_mediaCommand(command, data) {
    var media = this.target;

    switch (command) {
      case "play":
        media.play(); <-- Causes CPOW warning
        break;
      case "pause":
        media.pause(); <-- Causes CPOW warning
        break;
      case "mute":
        media.muted = true; <-- Causes CPOW warning
        break;
      case "unmute":
        media.muted = false; <-- Causes CPOW warning
        break;
      case "playbackRate":
        media.playbackRate = data; <-- Causes CPOW warning
        break;
      case "hidecontrols":
        media.removeAttribute("controls"); <-- Causes CPOW warning
        break;
      case "showcontrols":
        media.setAttribute("controls", "true"); <-- Causes CPOW warning
        break;
      case "hidestats":
      case "showstats":
        var event = media.ownerDocument.createEvent("CustomEvent"); <-- Causes CPOW warning
        event.initCustomEvent("media-showStatistics", false, true, command == "showstats"); <-- Causes CPOW warning
        media.dispatchEvent(event); <-- Causes CPOW warning
        break;
    }
  },
I'm not sure which tests to select for a try run.
Assignee: nobody → moz-ian
Status: NEW → ASSIGNED
Attachment #8566436 - Flags: review?(gavin.sharp)
Blocks: 1134409
Attachment #8566436 - Flags: review?(gavin.sharp) → review?(mconley)
Comment on attachment 8566436 [details] [diff] [review]
Make context-menu media commands e10s safe

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

This looks really good - going to test it before I r+. Just one nit.

::: browser/base/content/content.js
@@ +1066,5 @@
>  });
> +
> +addMessageListener("ContextMenu:MediaCommand", (message) => {
> +  let media = message.objects.element;
> +  switch (message.data.command) {

Nit, let's put a newline before this switch
Beyond the nit, this looks good. Local testing is solid with e10s and not, and I definitely don't see any unsafe CPOW usage warnings when I use the controls. Nice job!


Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d339d79519b9
Attachment #8566436 - Attachment is obsolete: true
Attachment #8566436 - Flags: review?(mconley)
Comment on attachment 8567108 [details] [diff] [review]
Make context-menu media commands use messages to avoid unsafe CPOW usage warnings

Try looks good - I've updated the commit message and added the newline.

Thanks Ian!
Attachment #8567108 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/2febda909f2d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 38
QA Whiteboard: [good first verify][verify in Nightly only]
You need to log in before you can comment on or make changes to this bug.