1453668 - Share popup flickers when hovering over it [Tunein]

Status: VERIFIED FIXED

(Core :: Web Painting, defect, P1)

Description

[David Olah]

[Affected versions]: 
Nightly 61.0a1 32 bit

[Affected platforms]:
Windows 7 x 64, Windows 10 x64, Mac OS X 10.13, Ubuntu 16.04

[Steps to reproduce]:

Only for Circle and Ellipse
1. Go to Ex: https://tunein.com/radio/NBA-on-TuneIn-s262441/
2. Click the share button from the bottom of the page (Between sound and favourite buttons)
3. Hover the mouse over the share popup

[Expected result]:
- The popup doesn't flicker

[Actual result]:
- The popup flickers (A black box flickers instead of the popup)

[Regression range]:
I narrowed the regression range with mozregression and this is the resulted pushlog URL: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=dfc311c89e45ead7e975d45b1643afe7ff160ac8&tochange=9bad112e0374d2661a2cb9d2fab2c957e847c093

Comment 1

[David Olah]

Mike, do you think this issue is a regression from Bug 1447193?

Comment 2

[Emilio Cobos Álvarez (:emilio)]

(In reply to David Olah from comment #1)
> Mike, do you think this issue is a regression from Bug 1447193?

Unlikely.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

I was kinda able to reproduce this on my Windows box. I didn't see a black box in the popup, but the gray from the overlay over the rest of the content seemed to overlap the popup sometimes when hovering.

mozregression pointed at bug 1450360 which seems like a more likely culprit.

Comment 4

[Matt Woodrow (:mattwoodrow)]

This is a variation of bug 1453541.

The frame tree looks roughly like:

Placeholder(Fixed)
Fixed
  Transformed - modified
  Block
    Placeholder(Fixed2)
Fixed2

The transformed frame is modified, and we store the dirty region on the nearest stacking context ancestor, Fixed. We don't do anything with Fixed2, since it's not modified, nor are any of it's content ancestors.

When we build, we do MarkFramesForDisplay on the fixed elements, but Fixed2 is neither modified, nor intersects the building area for the root stacking context (which is empty).

We descend into Fixed, load the stored dirty region which covers Transformed and build that. We don't build Block, since it has a 0 size, and thus doesn't intersect the dirty region. We'd normally have set the flag to descend through it to Fixed2, but we didn't, since we thought Fixed2 didn't intersect (since we checked against the dirty area for the root, not this stacking context).

Since we invalidated Transformed, but didn't rebuild Fixed2, we no longer have the relative ordering information available, and merging can end up putting the overlay content (Transformed) behind the black-out (Fixed2). Visually that looks like a flash.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1453668 - Restrict the retained display list stacking context optimization to frames that are also containing blocks for position:fixed.

This makes the optimization rarer, but is significantly simpler, since we should now be guaranteed that all placeholder frames have their out of flow frames in the same stacking context.

Review commit: https://reviewboard.mozilla.org/r/236816/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/236816/

Comment 6

[Miko Mynttinen]

Comment on attachment 8968135 [details]
Bug 1453668 - Restrict the retained display list stacking context optimization to frames that are also containing blocks for position:fixed.

https://reviewboard.mozilla.org/r/236816/#review242682

LGTM.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b584ff6f5afa
Restrict the retained display list stacking context optimization to frames that are also containing blockks for position:fixed. r=miko

Comment 8

[Stefan Hindli [:stefan_hindli]]

Backed out changeset b584ff6f5afa (bug 1453668) for R failures in file:///C:/Users/task_1523893739/build/tests/reftest/tests/layout/reftests/display-list/retained-dl-style-change-stacking-context-1.html on a CLOSED TREE

Problematic push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b584ff6f5afa5010f52016832e598e712288b47e&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified
Failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=173915275
Backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=aa20ed5dbcbdcacb559fcee3d44b4715d4528c99&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified
Log: https://treeherder.mozilla.org/logviewer.html#?job_id=173915275&repo=autoland&lineNumber=8322

Relevant part:
16:58:22    ERROR -  REFTEST TEST-UNEXPECTED-FAIL | file:///C:/Users/task_1523897362/build/tests/reftest/tests/layout/reftests/mathml/font-inflation-1.html == file:///C:/Users/task_1523897362/build/tests/reftest/tests/layout/reftests/mathml/font-inflation-1-ref.html | image comparison, max difference: 255, number of differing pixels: 220

Comment 9

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8968135 [details]
Bug 1453668 - Restrict the retained display list stacking context optimization to frames that are also containing blocks for position:fixed.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/236816/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/229be2ad5ee3
Restrict the retained display list stacking context optimization to frames that are also containing blocks for position:fixed. r=miko

Comment 11

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/229be2ad5ee3

Comment 12

[Rares Doghi, Desktop QA]

Hello, I have re-tested this issue and it no longer occurs in version: 61.0a1 (2018-04-26) on Ubuntu 16.04 as well as Windows 10 and Mac OS 10.13.
Confirmed as Fixed.