Closed Bug 1127794 Opened 7 years ago Closed 7 years ago

Windowed plugin origin doesn't always match rendered content after a scroll

Categories

(Core :: Plug-ins, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
e10s m5+ ---
firefox39 --- fixed

People

(Reporter: jimm, Assigned: jimm)

References

Details

Attachments

(3 files, 2 obsolete files)

STR:

1) load test case and let flash load completely
2) scroll the page up and then back to the original stop at the top of the page

result:

Sometimes the plugin window never makes it back to its original origin in the page. I've dumped these coordinates in debug mode and can confirm that the last layer update with plugin data holds coordinates that are slightly out of sync.

A few potential causes:

1) the location of the final viewport the compositor chooses in the parent doesn't match the information sent to the content process when reflow was triggered.

2) It might have something to do with this weird delayed plugin geometry update timer [1] we have for non-e10s.

3) ??

Roc suggested in the original compositor / plugin work that we might have problems with pushing plugin data over from layout in the content process, and that it might behoove us to use an actual layer injected into the layer tree for plugin position calculations. This solution might help here.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2933
Attached file flash test case
Attached file test plugin test case
Blocks: 1133237
This bug wasn't as bad as I thought. Currently we collect up plugin geometry updates for content a little too late for the updates to get shipped over with the shadow layer updates, which occurs in nsDisplayList::PaintRoot. We need to make sure we've computed and collected the updates before the call to EndTransaction on the layer manager.
Attached patch patch (obsolete) — Splinter Review
Hey roc, do you remember why you introduced this geometry timer?

http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#2923

From InitApplyPluginGeometryTimer -

"paints might get optimized away if the old plugin geometry covers the invalid region"

and somehow making a delayed call to ApplyPluginGeometryUpdates fixes this. I'm wondering if we need to get this hooked up in e10s.
Flags: needinfo?(roc)
Duplicate of this bug: 1128079
Attached patch patch (obsolete) — Splinter Review
also please see comment 6.
Attachment #8570704 - Attachment is obsolete: true
Flags: needinfo?(roc)
Attachment #8571033 - Flags: review?(roc)
Blocks: 1137944
(In reply to Jim Mathies [:jimm] from comment #6)
> Hey roc, do you remember why you introduced this geometry timer?
> 
> http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.
> cpp#2923
> 
> From InitApplyPluginGeometryTimer -
> 
> "paints might get optimized away if the old plugin geometry covers the
> invalid region"
> 
> and somehow making a delayed call to ApplyPluginGeometryUpdates fixes this.
> I'm wondering if we need to get this hooked up in e10s.

Consider the case where a plugin window covers the entire content area and then the plugin is supposed to move. We would request a paint by invalidating some area of the content, but that invalidation would be suppressed by Windows since it's still covered by the plugin's old window area, so we'd never get A WM_PAINT, never get around to painting, and hence never get around to moving the plugin window.

I don't think this should be a problem anymore since we're not relying on Windows' WM_PAINT events to drive painting.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9)
> (In reply to Jim Mathies [:jimm] from comment #6)
> > Hey roc, do you remember why you introduced this geometry timer?
> > 
> > http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.
> > cpp#2923
> > 
> > From InitApplyPluginGeometryTimer -
> > 
> > "paints might get optimized away if the old plugin geometry covers the
> > invalid region"
> > 
> > and somehow making a delayed call to ApplyPluginGeometryUpdates fixes this.
> > I'm wondering if we need to get this hooked up in e10s.
> 
> Consider the case where a plugin window covers the entire content area and
> then the plugin is supposed to move. We would request a paint by
> invalidating some area of the content, but that invalidation would be
> suppressed by Windows since it's still covered by the plugin's old window
> area, so we'd never get A WM_PAINT, never get around to painting, and hence
> never get around to moving the plugin window.
> 
> I don't think this should be a problem anymore since we're not relying on
> Windows' WM_PAINT events to drive painting.

Excellent, thanks for the update.
Attachment #8571033 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/702f6a2260ed
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Duplicate of this bug: 1133235
You need to log in before you can comment on or make changes to this bug.