Closed Bug 1196539 Opened 9 years ago Closed 9 years ago

[e10s]  After switching the tab, flash plug-in would not repaint until mouse hover the plug-in or scrollbar

Categories

(Core Graveyard :: Plug-ins, defect)

43 Branch
Unspecified
All
defect
Not set
normal

Tracking

(e10sm8+, firefox44 fixed)

RESOLVED FIXED
mozilla44
Tracking Status
e10s m8+ ---
firefox44 --- fixed

People

(Reporter: alice0775, Assigned: jimm)

References

Details

(Keywords: perf, regression)

Attachments

(5 files, 2 obsolete files)

Steps to reproduce:
1. Open 2 tabs
   about:home and http://edition.cnn.com/2014/11/23/showbiz/music/katy-perry-super-bowl/index.html?hpt=en_bn1
2. Wait for the video autoplay
3. Switch between tabs

Actual Results:
Flush plug-in would not repaint until mouse hover the plug-in or scrollbar

Expected results:
The flash plugin should be repainted immediately after switching tab

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=f00b7bf7b9f83d74ca8413d04da4942c8bcc84db&tochange=0f5421c28b14

Suspect:  Bug 1163570
Attached file screencap zipped ogg
Assignee: nobody → jmathies
Attached file dance.swf
Attached file testcase html
Steps to reproduce:
1. Open 2 tabs
   about:home and the attached testcase
2. Wait for the Flash autoplay
3. Switch between tabs

Actual Results:
Flush plug-in would not repaint until mouse hover the scrollbar or resize window

Expected results:
The flash plugin should be repainted immediately after switching tab
This problem was also reproduced on ubuntu14.04
https://hg.mozilla.org/mozilla-central/rev/095988abdc560bf8ba07a94a425c6922a3e9bfd6
Mozilla/5.0 (X11; Linux i686; rv:43.0) Gecko/20100101 Firefox/43.0 ID:20150821030204
OS: Windows 7 → All
You don't need the html test case really, just load the swf directly.
I can fix this, but it's going to relay on some code in bug 1137944.
Depends on: 1137944
[Tracking Requested - why for this release]:

[Tracking Requested - why for this release]:

This seems likely to affect Firefox 40 and onwards if the culprit is bug 1163570.
Summary: [e10s]  After switching the tab, flush plug-in would not repaint until mouse hover the plug-in or scrollbar → [e10s]  After switching the tab, flash plug-in would not repaint until mouse hover the plug-in or scrollbar
bug 1163570 is only in 43 at this point.
Flags: needinfo?(lhenry)
Thanks Jim, I misread version reported vs. target milestone!
Flags: needinfo?(lhenry)
I also see this problem playing Silverlight video on Netflix. The plugin tab does not repaint, but I can still hear Silverlight audio playing. If you switch back and forth between the tabs, the plugin tab will sometimes repaint.

I can reproduce this in a Windows 8.1 VM and Windows 10 VM, but not OS X. I don't have a real Windows machine to test. Maybe this is a VM painting issue?

I can only reproduce the problem when e10s is enabled.
(In reply to Chris Peterson [:cpeterson] from comment #11)
> I also see this problem playing Silverlight video on Netflix. The plugin tab
> does not repaint, but I can still hear Silverlight audio playing. If you
> switch back and forth between the tabs, the plugin tab will sometimes
> repaint.
> 
> I can reproduce this in a Windows 8.1 VM and Windows 10 VM, but not OS X. I
> don't have a real Windows machine to test. Maybe this is a VM painting issue?
> 
> I can only reproduce the problem when e10s is enabled.

Thanks for the details report. This is related to a problem with compositor code that manages plugin windows.
More reliable fix than the checks I was using for roots and such via composition events.
Attached patch tests (obsolete) — Splinter Review
Comment on attachment 8658437 [details] [diff] [review]
use AutoResolveRefLayers logic to update plugins

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

Matt, mind looking this over?
Attachment #8658437 - Flags: review?(matt.woodrow)
Attachment #8658439 - Flags: review?(matt.woodrow)
fixes a build failure due to an unused variable.
Attachment #8658437 - Attachment is obsolete: true
Attachment #8658437 - Flags: review?(matt.woodrow)
Attachment #8658668 - Flags: review?(matt.woodrow)
Comment on attachment 8658668 [details] [diff] [review]
use AutoResolveRefLayers logic to update plugins

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

::: gfx/layers/ipc/CompositorParent.cpp
@@ +1987,5 @@
>  }
>  
>  #if defined(XP_WIN) || defined(MOZ_WIDGET_GTK)
> +// static, sends plugin window state changes to the main thread
> +void 

Trailing whitespace
Attachment #8658668 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8658439 [details] [diff] [review]
tests

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

I'm not really the best person to review JS test additions.

::: dom/plugins/test/testplugin/nptest_windows.cpp
@@ +50,5 @@
>    HWND childWindow;
>    ID3D10Device1 *device;
>    ID3D10Texture2D *frontBuffer;
>    ID3D10Texture2D *backBuffer;
> +  uint32_t showWindowCount;

What is this added for? Doesn't seem to be used anywhere.
Attachment #8658439 - Flags: review?(matt.woodrow)
(In reply to Matt Woodrow (:mattwoodrow) from comment #19)
> Comment on attachment 8658439 [details] [diff] [review]
> tests
> 
> Review of attachment 8658439 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not really the best person to review JS test additions.
> 
> ::: dom/plugins/test/testplugin/nptest_windows.cpp
> @@ +50,5 @@
> >    HWND childWindow;
> >    ID3D10Device1 *device;
> >    ID3D10Texture2D *frontBuffer;
> >    ID3D10Texture2D *backBuffer;
> > +  uint32_t showWindowCount;
> 
> What is this added for? Doesn't seem to be used anywhere.

sorry, that was bleed through from another test patch. I'll bounce this off roc, he already reviewed a couple of these for me.
Attached patch added testsSplinter Review
Attachment #8658439 - Attachment is obsolete: true
Comment on attachment 8658750 [details] [diff] [review]
added tests

more plugin + compositor tests
Attachment #8658750 - Flags: review?(roc)
Blocks: 1137944
No longer depends on: 1137944
Depends on: 1258058
See Also: → 1271898
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: