Closed Bug 1152326 Opened 5 years ago Closed 5 years ago

[e10s] Flash plugin is continuously flickering if multiple browsers are opened

Categories

(Core :: Graphics: Layers, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
e10s m7+ ---
firefox41 --- fixed

People

(Reporter: alice0775, Assigned: jimm)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: gfx-noted)

Attachments

(1 file, 1 obsolete file)

This happens on e10s only.
This is similar to Bug 1151319, but regression range is different.

Steps to reproduce:
1. Open http://game.goo.ne.jp/choi/title/solitaire/index.html in window[A]
2. Open new window[B] and about:home

Actual results:
The Flash is continuously flashing

Expected results:
Not flashing


#1 Regression window:
The Flash completely disappears
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c6d4680f1f5a&tochange=1968388d42c2
Triggered by: 3eb6f6c017ce	Jim Mathies — Bug 1133237 - When transitioning from a shadow layer tree that has plugins to a tree that does not, make sure the old plugins get hidden properly. r=roc


#2 Regression window:
The Flash is continuously flickering
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4e6629bce17b&tochange=fdf4671db324
Triggered by: ddd5517ce7d2	Jim Mathies — Bug 1138181 - Be more aggressive in updating plugin geometry in the compositor, avoids filtering out important offset updates that don't involve remote layer tree updates. r=roc
Flags: needinfo?(jmathies)
See Also: → 1151319
The needinfos keep these out of our triage lists.
Flags: needinfo?(jmathies)
Blocks: 1154095
Blocks: e10s-plugins
Assignee: nobody → jmathies
Duplicate of this bug: 1151319
Blocks: 1167978
I'm not able to reproduce this having the two windows open and sitting next to each other on the desktop.
Oh, I think I just got this briefly, looking.
I see this all the time on a bit slower laptop (Win7).
Attached patch patch v.1 (obsolete) — Splinter Review
The widget visibility routines were being asked to set all plugins not included with layer updates to hidden. Since we have one compositor per top level window each compositor was trying to the plugin windows owned by other top level windows to hidden.

This is fixes the problem - basically hand the parent widget in to the visibility routines and check it against the parent of the widget before calling Show.
Attached patch patch v.1Splinter Review
Attachment #8623275 - Attachment is obsolete: true
Attachment #8623280 - Flags: review?(aklotz)
No longer blocks: 1167978
Comment on attachment 8623280 [details] [diff] [review]
patch v.1

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

::: gfx/layers/ipc/CompositorChild.cpp
@@ +270,5 @@
>        NS_WARNING("Unexpected, plugin id not found!");
>        continue;
>      }
> +    if (!parent) {
> +      parent = widget->GetParent();

What if widget changes and has a different parent than the one saved here? Is that possible?
(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #8)
> Comment on attachment 8623280 [details] [diff] [review]
> patch v.1
> 
> Review of attachment 8623280 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/ipc/CompositorChild.cpp
> @@ +270,5 @@
> >        NS_WARNING("Unexpected, plugin id not found!");
> >        continue;
> >      }
> > +    if (!parent) {
> > +      parent = widget->GetParent();
> 
> What if widget changes and has a different parent than the one saved here?
> Is that possible?
Flags: needinfo?(jmathies)
(In reply to Aaron Klotz [:aklotz] (please use needinfo) from comment #8)
> Comment on attachment 8623280 [details] [diff] [review]
> patch v.1
> 
> Review of attachment 8623280 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/ipc/CompositorChild.cpp
> @@ +270,5 @@
> >        NS_WARNING("Unexpected, plugin id not found!");
> >        continue;
> >      }
> > +    if (!parent) {
> > +      parent = widget->GetParent();
> 
> What if widget changes and has a different parent than the one saved here?
> Is that possible?

Plugin updates here are grouped by top level window since compositors are tied to top level windows. So every one of these plugins should have the same parent. If the parent changes at some point (tab drag out for example) it shouldn't effect this code at all since the parents should all get updated.
Flags: needinfo?(jmathies)
Comment on attachment 8623280 [details] [diff] [review]
patch v.1

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

OK, thanks for the info. LGTM.
Attachment #8623280 - Flags: review?(aklotz) → review+
https://hg.mozilla.org/mozilla-central/rev/24c16a06692e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Duplicate of this bug: 1154095
You need to log in before you can comment on or make changes to this bug.