Closed Bug 1241060 Opened 8 years ago Closed 8 years ago

[e10s] Flash Player content appears on top of devtools when I open Network tab

Categories

(Core Graveyard :: Plug-ins, defect)

defect
Not set
normal

Tracking

(e10sm9+, firefox46 wontfix, firefox47 fixed, firefox48 fixed)

VERIFIED FIXED
mozilla48
Tracking Status
e10s m9+ ---
firefox46 --- wontfix
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: arni2033, Assigned: jimm)

References

()

Details

Attachments

(4 files, 3 obsolete files)

>>>   My Info:   Win7_64, Nightly 46, 32bit, ID 20160118030338
STR:
1. Open the following "data:" url or click URL in the form above
>   data:text/html,<iframe src="https://www.youtube.com/v/60og9gwKh1o" height="344" width="425"></iframe><style>body{height:10000px;}
2. Make sure that it's Flash that is displayed in iframe (not html5 stuff)
3. Open Netmonitor (Ctrl+Shift+Q), dock it to the bottom side of window.
   Resize the toolbox so that the Flash was fully visible, close devtools
4. Open Netmonitor, resize devtools toolbox so that it overlapped Flash content on the page
5. Move mouse over the free space on the page (white area),
   hold middle mouse button (i.e. mouse wheel) and move mouse to the bottom to start autoscrolling
6. Release middle mouse button

Result:       
 Step 4 fails: Flash content overlaps the devtools toolbox!
 During Step 5 Flash content stays on its place on the screen
 After Step 6 Flash content is still displayed on the same place.
   This is NOT just a "redraw" thing like bug 1148978, bug 1137944. See screencast.

Expectations: 
 The same as non-e10s: Flash content should (1) be displayed under devtools toolbox
                                            (2) move simultaneously with the rest of page content
NI?jimm since he's been working on related windowed-Flash issues.
tracking-e10s: --- → ?
Flags: needinfo?(jmathies)
Simple str:

1) open a windowed flash test case
2) open dev tools
3) resize dev tool up to where the plugin window is

result: dev tools is obscured by the window

We're not triggering clipping calculations on the plugin window. The clipping does get updated for other chrome related changes like sidebars or the shifting of chrome from menu display. Dev tools though doesn't get picked up.

Marcus, any ideas here?
Flags: needinfo?(jmathies) → needinfo?(mstange)
I don't have any ideas.
Flags: needinfo?(mstange)
This behavior is actually very similar to bug 1229164 + bug 1229455
See Also: → 1229455
We bail out of our visibility calculations here - 

https://dxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.cpp#1064

sibling->GetLocalTransform().Is2D(&siblingMatrix) is true.
(In reply to Jim Mathies [:jimm] from comment #5)
> We bail out of our visibility calculations here - 
> 
> https://dxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.cpp#1064
> 
> sibling->GetLocalTransform().Is2D(&siblingMatrix) is true.

sorry, its false. So we run into a sibling matrix that isn't 2D. I'm not sure why.
Assignee: nobody → jmathies
Attached patch patch (obsolete) — Splinter Review
I'm not entirely sure what this sibling layer is. I know it's in the dev tools panel and the base object is a ContainedLayerComposite. It does not have a 2D transformation matrix and it has zero bounds.

I've added a check here to ignore siblings that aren't visible which avoids this early return and fixes this bug. Roc, what do you think? I can debug this thing a bit further if you like to try and figure out what element it's associated with.
Attachment #8710633 - Flags: review?(roc)
Comment on attachment 8710633 [details] [diff] [review]
patch

actually that doesn't quite look right. Let me check this again.
Attachment #8710633 - Flags: review?(roc)
Can you print out the values of the matrix? What kind of matrix is it?
Attached patch patch (obsolete) — Splinter Review
better patch, last one had a type, and the IsVisible check didn't pick up zero bounds for some reason.
Attachment #8710637 - Attachment is patch: true
Attachment #8710633 - Attachment is obsolete: true
Attachment #8710637 - Flags: review?(roc)
Attached patch patch (obsolete) — Splinter Review
bah, not having a good bugzilla day.
Attachment #8710637 - Attachment is obsolete: true
Attachment #8710637 - Flags: review?(roc)
Attachment #8710639 - Flags: review?(roc)
(In reply to Markus Stange [:mstange] from comment #9)
> Can you print out the values of the matrix? What kind of matrix is it?

I checked, it's an identity matrix.
How is it not 2D then? Is it maybe just slightly not 2d, but not by much? You could try calling NudgeTo2D.
Comment on attachment 8710639 [details] [diff] [review]
patch

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

I think we should figure out what's causing this. A display list+layers dump should show the offending layer and which display item(s) are part of it.
Attachment #8710639 - Flags: review?(roc)
Attached file layer dump
looks like this is caused by the entire dev tools panel. attached is a layer dump.
my matrix math is a bit rusty but this looks like a botched translation matrix. 

1 0 0 0
0 1 0 0
0 0 1 0
1 984 1 1

In this particular dump the dev tool panel was 984 pixels offset from the window origin. Roc, any ideas here?
Flags: needinfo?(roc)
Yes that does look weird. I have no idea how we got into that state. Should be debuggable.
Flags: needinfo?(roc)
Blocks: 1229455
Blocks: 1245994
Blocks: 1245995
No longer blocks: 1245994
No longer blocks: 1229455
Attached file stack.txt
The stack posted here is where the dev panel's local transform comes from. It's not 2d and it's a translation. When we come across this in GetVisibleRegionRelativeToRootLayer [1] we bail and skip clipping the plugin window. It looks like this is expected and as such the flaw is in these sibling checks we're doing. I think the answer here is to skip over the sibling layer when we run into this vs. avoiding setting clipping all together.

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/layers/Layers.cpp#1017
What are the coordinates that are passed to PostTranslate? Where do they come from?
Even if we may be able to work around a 3d transform here, I still think having a 3d transform here in the first place is a mistake.
Comment on attachment 8732873 [details]
MozReview Request: Bug 1241060 - Skip non-2d sibling layers when calculating pluginw window clipping under e10s. r?mstange

https://reviewboard.mozilla.org/r/41429/#review37951
Attachment #8732873 - Flags: review?(mstange) → review+
Attachment #8710639 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/70e7c1da43c0
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Fixed on:   Win7_64, Nightly 48, 32bit, ID 20160324030447
Status: RESOLVED → VERIFIED
Jim, should this fix be uplifted to Aurora 47 in preparation for our e10s experiments on Beta 47?
Flags: needinfo?(jmathies)
Comment on attachment 8732873 [details]
MozReview Request: Bug 1241060 - Skip non-2d sibling layers when calculating pluginw window clipping under e10s. r?mstange

Approval Request Comment
[Feature/regressing bug #]:
e10s issue
[User impact if declined]:
polishy, windowed plugins can overlap dev tools. not a big deal.
[Describe test coverage new/current, TreeHerder]:
on mc for a week.
[Risks and why]: 
low, well understood change.
[String/UUID change made/needed]:
none.
Flags: needinfo?(jmathies)
Attachment #8732873 - Flags: approval-mozilla-aurora?
Comment on attachment 8732873 [details]
MozReview Request: Bug 1241060 - Skip non-2d sibling layers when calculating pluginw window clipping under e10s. r?mstange

Fix was verified, e10s related, Aurora47+
Attachment #8732873 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
No longer blocks: 1245995
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.