|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
Steps to reproduce: 1. Open http://zombo.com/ in a tab. 2. Switch to a different tab that does not have a plugin in it. Expected results: The plugin background tab should not cause any calls to PPluginInstance::SendShow. Actual results: The plugin keeps submitting frames at 60fps. Example profile: https://clptr.io/2iyM9Om
This is because we're not setting the clipRect to empty.
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Comment on attachment 8827521 [details] Bug 1331402 - Update plugins' clipRects when hiding a tab. https://reviewboard.mozilla.org/r/105182/#review106318
Attachment #8827521 - Flags: review?(jmathies) → review+
Comment on attachment 8827521 [details] Bug 1331402 - Update plugins' clipRects when hiding a tab. https://reviewboard.mozilla.org/r/105182/#review106482
Attachment #8827521 - Flags: review?(matt.woodrow) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
is this safe to uplift to 52 (beta as of today)?
I'd like to uplift it but I'm not 100% sure how / whether this bug and the patch affect non-Mac platforms, so I'm not comfortable requesting uplift quite yet. Can you check whether plugins on Windows are affected by it?
Flags: needinfo?(mstange) → needinfo?(jmathies)
(In reply to Markus Stange [:mstange] from comment #7) > I'd like to uplift it but I'm not 100% sure how / whether this bug and the > patch affect non-Mac platforms, so I'm not comfortable requesting uplift > quite yet. Can you check whether plugins on Windows are affected by it? Hmm, so we update clipRect for both windowed and windowless on Windows in a slightly different place - xul.dll!nsPluginInstanceOwner::UpdateWindowPositionAndClipRect(bool aSetWindow) Line 3625 C++ xul.dll!nsPluginInstanceOwner::UpdateDocumentActiveState(bool aIsActive) Line 3689 C++ xul.dll!SetPluginIsActive(nsISupports * aSupports, void * aClosure) Line 10728 C++ xul.dll!nsIDocument::EnumerateActivityObservers(void(*)(nsISupports *, void *) aEnumerator, void * aData) Line 9779 C++ xul.dll!mozilla::PresShell::SetIsActive(bool aIsActive) Line 10750 C++ xul.dll!nsDocShell::SetIsActive(bool aIsActive) Line 6198 C++ xul.dll!mozilla::dom::TabChild::RecvSetDocShellIsActive(const bool & aIsActive, const bool & aPreserveLayers, const unsigned __int64 & aLayerObserverEpoch) Line 2424 C++ So on Windows I think your added code is redundant. However it doesn't result in any additional calls to SetWindow on the plugin since we have checks for this in nsPluginInstanceOwner::UpdateWindowPositionAndClipRect. I haven't check yet but I'[m pretty sure linux follows this code path as well. Maybe we should ifdef that code you added to osx?
Flags: needinfo?(jmathies) → needinfo?(mstange)
or maybe move it to the nsPluginFrame code specific to OSX, assuming there's something in there that gets triggered by the doc active state changes.
Or we could make nsPluginInstanceOwner::UpdateDocumentActiveState call into similar code on OSX (which sets the clip rect to empty) and remove the code I added in this bug. That would make things work more similarly on the different platforms.
You need to log in before you can comment on or make changes to this bug.