Closed Bug 1452805 Opened 6 years ago Closed 6 years ago

regression: Broken menu on Twitter

Categories

(Core :: Web Painting, defect, P1)

x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + verified

People

(Reporter: jan, Assigned: mattwoodrow)

References

()

Details

(Keywords: correctness, nightly-community, regression)

Attachments

(2 files)

Attached video 2018-04-10_00-19-06.mp4
(Mozregression is confused, so I had to take this way.)
mozregression --repo mozilla-inbound --good 2018-04-03 --bad 2018-04-05 --pref gfx.webrender.all:true gfx.webrender.blob.invalidation:true startup.homepage_welcome_url:"https://twitter.com/FxSiteCompat"
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7c2423c6e9d0d22b1db406db1f81fbab2a0b02ef&tochange=dd31fb345c11732432fa61a7d8a1b727d41b4a1c
> 
> 8:52.52 INFO: ************* Switching to mozilla-central
Ctrl + C

mozregression --repo autoland --good 6c03cf8c9e8e --bad 479b6858ea6c --pref gfx.webrender.all:true gfx.webrender.blob.invalidation:true startup.homepage_welcome_url:"https://twitter.com/FxSiteCompat"
> 5:30.72 INFO: Last good revision: 6415ccbf739f36f71548540bd92c3b86ab9e1529
> 5:30.72 INFO: First bad revision: d112cf7b2b60e6244099dc3b599a2444ba0d1da3
> 5:30.72 INFO: Pushlog:
> https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=6415ccbf739f36f71548540bd92c3b86ab9e1529&tochange=d112cf7b2b60e6244099dc3b599a2444ba0d1da3

probably:
> 99de9f5450d8	Matt Woodrow — Bug 1443027 - Fix the merging algorithm to pass the new tests correctly. r=mstange
> 373a7a3e5c8f	Matt Woodrow — Bug 1443027 - Add two new tests for merging behaviour. r=mstange

mozregression --repo autoland --launch d112cf7b2b60e6244099dc3b599a2444ba0d1da3 --pref startup.homepage_welcome_url:"https://twitter.com/FxSiteCompat"
non-WR is also bad.
Flags: needinfo?(matt.woodrow)
Does this still reproduce for you with nightlies that include the fix for bug 1451971?
Flags: needinfo?(jan)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #1)
> Does this still reproduce for you with nightlies that include the fix for bug 1451971?

Yes.

(Stefan Hindli [:stefan_hindli] from bug 1451971 comment 10)
> https://hg.mozilla.org/mozilla-central/rev/79a5ebb7f796
> https://hg.mozilla.org/mozilla-central/rev/6a8df72533ba

mozregression --launch 6a8df72533ba --pref startup.homepage_welcome_url:"https://twitter.com/FxSiteCompat"
bad

Latest push from https://hg.mozilla.org/mozilla-central/pushloghtml:
https://hg.mozilla.org/mozilla-central/graph/88f297d206a6
mozregression --launch 88f297d206a6 --pref startup.homepage_welcome_url:"https://twitter.com/FxSiteCompat"
bad
Flags: needinfo?(jan)
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Priority: -- → P1
I can reproduce this, got a fix locally.

Will write a test in the morning and get the patch up.
Comment on attachment 8967583 [details]
Bug 1452805 - Make sure we rebuild contents infront and behind stacking contexts if their size might have changed.

https://reviewboard.mozilla.org/r/236278/#review242558

LGTM.
Attachment #8967583 - Flags: review?(mikokm) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s dc56ecc422995817c6e5847a5c7295ba9a42b9c3 -d 212963ca6f2b: rebasing 458542:dc56ecc42299 "Bug 1452805 - Make sure we rebuild contents infront and behind stacking contexts if their size might have changed. r=miko" (tip)
merging layout/painting/RetainedDisplayListBuilder.cpp
merging layout/reftests/display-list/reftest.list
warning: conflicts while merging layout/reftests/display-list/reftest.list! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ee5f61737378
Make sure we rebuild contents infront and behind stacking contexts if their size might have changed. r=miko
Backout by apavel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f902138acc12
Backed out changeset ee5f61737378 for failing  awsy/test_memory_usage.py on a CLOSED TREE
Backed out for failing in multiple test suits such as browser/base/content/test/about/browser_aboutHome_search_composing.js,  awsy/test_memory_usage.py, layout/reftests/svg/smil/transform/scale-1.svg, devtools/client/inspector/test/browser_inspector_highlighter-cssshape_01.js

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ee5f617373788a8ed0f20b11994679cdecf9ea96

Failure logs: https://treeherder.mozilla.org/logviewer.html#?job_id=173881483&repo=autoland&lineNumber=804
https://treeherder.mozilla.org/logviewer.html#?job_id=173879670&repo=autoland&lineNumber=3264
https://treeherder.mozilla.org/logviewer.html#?job_id=173883531&repo=autoland&lineNumber=2097

Backout: https://hg.mozilla.org/integration/autoland/rev/f902138acc12ddd5fd819f5bf2a9b962f4451370
Flags: needinfo?(matt.woodrow)
The original bug was because opacity stacking contexts don't clip the visible regions from the build (the display list building area), so the visible region on the stacking context covered the whole window, even though the contents of the stacking context are small. This made the size change check never fail.

The first fix failed because GetBounds() isn't cached, and it was recomputing bounds based on the children during ProcessFrame. This is bad, since it can read the new overflow area from frames that have changed, and it crashed because some items can have a deleted frame.

If we clip the visible rect to the overflow rect for frames that might use this optimization (transform already does it), then it works correctly.

Can't seem to get MozReview to let me request review again, so ni? instead.
Flags: needinfo?(matt.woodrow) → needinfo?(mikokm)
Comment on attachment 8967583 [details]
Bug 1452805 - Make sure we rebuild contents infront and behind stacking contexts if their size might have changed.

https://reviewboard.mozilla.org/r/236278/#review244178

::: layout/generic/nsFrame.cpp:2908
(Diff revision 2)
>          dirtyRect.SetEmpty();
>          visibleRect.SetEmpty();
>        }
>      }
>      inTransform = true;
> +  } else if (IsFixedPosContainingBlock()) {

Could you please add a comment here describing what this branch is doing, and why?

I think that it is not very obvious from the surrounding code.
Flags: needinfo?(mikokm)
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/070057638112
Make sure we rebuild contents infront and behind stacking contexts if their size might have changed. r=miko
https://hg.mozilla.org/mozilla-central/rev/070057638112
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Flags: qe-verify+
Managed to reproduce the issue on Nightly 61.0a1 (2018-04-05) on  Ubuntu 16.04 x64 as well as Windows 10 x64 and  Mac OS X 10.12.

On the latest Firefox Nightly (Build ID: 20180502100112) the issue cannot be reproduced on Mac OS X 10.12, Windows 10 x64 and Ubuntu 16.04 x64, thus I will close the issue as VERIFIED FIXED.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.