1540984 - Theme header background image flickers/blinks/flashes when changing/opening window

Status: VERIFIED FIXED

(WebExtensions :: Themes, defect, P2)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR:

1. Install some Theme
2. Open 2 browser windows
3. Change between them
   and see that theme flickers/blinks/flashes on black when changing/opening window

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: mozregression-gui.txt

Regression started in Mozilla Firefox Nightly 68.0a1 (2019-03-31).

Maybe it's caused by:
bug #1539026
bug #1525762
bug #1539925

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

opt and pgo builds searching is broken in mozregression-gui.
Only debug builds works properly.

Regression caused by:

Found commit message:
Bug 1539925: Part 4 - Remove stray mochitest install.rdf file. r=aswan
Differential Revision: https://phabricator.services.mozilla.com/D25292

Comment 3

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR2:

1. Open this website page:
   https://net-s.pl/produkt/amd-ryzen-7-2700-octo-core-320ghz-20mb-am4-65w-12nm-box-844443
2. Press CPU image to show bigger image in new window
   and see that theme flickers/blinks/flashes on black on background window when opening new foreground window

Comment 4

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Comment 5

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Attachment: screencast.mp4

Comment 6

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

Per comment #2 mozregression-gui pointing that this bug regressed by bug #1539925, not by bug #1525762. Is it wrong, that you change it?

Comment 7

[Tim Nguyen :ntim]

(In reply to Virtual_ManPL [:Virtual] - (please needinfo? me - so I will see your comment/reply/question/etc.) from comment #6)

Per comment #2 mozregression-gui pointing that this bug regressed by bug #1539925, not by bug #1525762. Is it wrong, that you change it?

From Kris’ new summary, bug 1525762 sounds more likely since that one actually removed the image cropper.

Comment 8

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

OK. Thank you very much for detailed information.

Comment 9

[David Durst [:ddurst]]

Does this happen on Windows between windows only, or between tabs as well?

Comment 10

[krupa raj [:krupa--use this to needinfo]]

cosmin, can you take a look?

Comment 11

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

@ David Durst [:ddurst] - Issue is reproducible for me only when I switch between windows, switching between tabs are unaffected.

Comment 12

[CosminB]

Attachment: Bug1540984.mp4

I can reproduce this issue on Firefox 68.0a1 (20190416220148) under Win 7 64-bit and Mac OS X 10.14.1

Please see the attached video.

(In reply to David Durst [:ddurst] from comment #9)

Does this happen on Windows between windows only, or between tabs as well?

This issue happens only between windows.

I did a regression window and observed that Bug 1525762 is the one that caused this regression.

INFO: Last good revision: 9b129ff965d75b364c9bdb4d737542cb4d1c40d1
INFO: First bad revision: c06dfc552c647a6ce96f35cd84c32a589dc85608
INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=9b129ff965d75b364c9bdb4d737542cb4d1c40d1&tochange=c06dfc552c647a6ce96f35cd84c32a589dc85608

Comment 13

[krupa raj [:krupa--use this to needinfo]]

I think we should try to fix this in 68 if possible.

Comment 14

[Julien Cristau [:jcristau]]

Mike, can we find someone to work on this?

Comment 15

[Tim Nguyen :ntim]

A backout of https://hg.mozilla.org/mozilla-central/rev/ba1529f7fd78 would suffice, but maybe Kris had something different in mind.

Comment 16

[Tim Nguyen :ntim]

Attachment: Bug 1540984 - Backed out changeset ba1529f7fd78 for causing flickers with large header themes.

Comment 17

[Tim Nguyen :ntim]

(In reply to Tim Nguyen :ntim from comment #15)

A backout of https://hg.mozilla.org/mozilla-central/rev/ba1529f7fd78 would suffice, but maybe Kris had something different in mind.

Oh wait, it will not work with static themes, since they use moz-extension:// images which are not covered by the image cropper. I'll leave this to someone else, since I don't have time or Windows 7 to test this.

Comment 18

[Kestrel]

I also see this on Ubuntu 19.04 when switching between multiple Firefox windows, so all platforms are affected.

Interestingly it does not happen when switching between private and non-private windows.

Comment 19

[Tim Nguyen :ntim]

(In reply to Kestrel from comment #18)

I also see this on Ubuntu 19.04 when switching between multiple Firefox windows, so all platforms are affected.

Interestingly it does not happen when switching between private and non-private windows.

I can't reproduce this on macOS, but presumably because it's ok with higher resolution images.

Comment 20

[Kestrel]

(In reply to Tim Nguyen :ntim from comment #19)

I can't reproduce this on macOS, but presumably because it's ok with higher resolution images.

Comment 12 mentions it occurring on Mac OS X 10.14.1.

Comment 21

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

@ Tim Nguyen :ntim - Please retry with updated STRs, maybe issue will finally be reproducible for you too.

Updated STRs:

STR1:

1. Install "Virtualfox" Theme ( https://addons.mozilla.org/addon/virtualfox/ )
2. Open 2 browser windows
3. Change between them
   and see that theme flickers/blinks/flashes on black when changing/opening window

STR2:

1. Install "Virtualfox" Theme ( https://addons.mozilla.org/addon/virtualfox/ )
2. Open this website page:
   https://net-s.pl/produkt/amd-ryzen-7-2700-octo-core-320ghz-20mb-am4-65w-12nm-box-844443
3. Press CPU image to show bigger image in new window
   and see that theme flickers/blinks/flashes on black on background window when opening new foreground window

Comment 22

[Tim Nguyen :ntim]

Yeah, I can reproduce now, thanks. I'm busy with other things though, so I'm not planning to take this.

Comment 23

[CosminB]

Attachment: MacOS.mov

Mac OS X 10.14.1

Comment 24

[Kris Maglione [:kmag]]

(In reply to Tim Nguyen :ntim from comment #15)

A backout of https://hg.mozilla.org/mozilla-central/rev/ba1529f7fd78 would suffice, but maybe Kris had something different in mind.

It would not.

Comment 25

[Mike de Boer [:mikedeboer]]

Since this was clearly regressed by bug 1525762, I think it'd be a good start for Kris to take a look to see if the image cropper can be re-jigged in a different way.
Or perhaps redirect top :aswan, who reviewed the patches(?)

Comment 26

[David Durst [:ddurst]]

I've asked for this side-effect to try to be fixed in 68. Product isn't that worried about it, and it's ultimately their call.

I've updated the summary because how to fix it should be up to the fixer; what to solve for is the requirement.

Comment 27

[Kris Maglione [:kmag]]

Attachment: Bug 1540984: Pre-load theme header image and share across all windows. r=aswan

For large header images, built-in memory caching does not work correctly, and
we wind up seeing a flicker any time a new window is opened or becomes active.
This patch caches the header image in an Image element at startup, and uses
that element as the backing for the header image in all browser windows.

Comment 28

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6159d01c9341088edcabb318259c8ce6201c61ac
Bug 1540984: Pre-load theme header image and share across all windows. r=aswan

Comment 29

[Natalia Csoregi [:nataliaCs]]

Backed out for assertion failures on Allocator.cpp:157

backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a722a039fb671037fa7570efb855c81ea9e3312e

push: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6159d01c9341088edcabb318259c8ce6201c61ac&group_state=expanded&selectedJob=244350089

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=244350066&repo=mozilla-inbound&lineNumber=54907

00:33:05 INFO - TEST-START | devtools/shared/webconsole/test/test_cached_messages.html
00:33:05 INFO - GECKO(2211) | ++DOMWINDOW == 43 (0x13074b800) [pid = 2211] [serial = 43] [outer = 0x131105020]
00:33:05 INFO - GECKO(2211) | console.log: "foobarBaz-log" (void 0)
00:33:05 INFO - GECKO(2211) | console.info: "foobarBaz-info" null
00:33:05 INFO - GECKO(2211) | console.warn: "foobarBaz-warn" ({})
00:33:05 INFO - GECKO(2211) | [2211, Main Thread] WARNING: Windowless browser was not closed prior to destruction: file /builds/worker/workspace/build/src/xpfe/appshell/nsAppShellService.cpp, line 410
00:33:05 INFO - GECKO(2211) | Assertion failure: cx->isNurseryAllocAllowed(), at /builds/worker/workspace/build/src/js/src/gc/Allocator.cpp:157
00:33:06 INFO - TEST-INFO | Main app process: exit 1
00:33:06 INFO - Buffered messages finished
00:33:06 ERROR - TEST-UNEXPECTED-FAIL | devtools/shared/webconsole/test/test_cached_messages.html | application terminated with exit code 1
00:33:06 INFO - runtests.py | Application ran for: 0:00:13.447991
00:33:06 INFO - zombiecheck | Reading PID log: /var/folders/61/88b5s4517kg5b0_xk4q4ylk000000x/T/tmpTZutOSpidlog
00:33:06 INFO - mozcrash Copy/paste: /Users/cltbld/tasks/task_1556842754/build/macosx64-minidump_stackwalk /var/folders/61/88b5s4517kg5b0_xk4q4ylk000000x/T/tmpPJmSLs.mozrunner/minidumps/568928C2-BD8A-48B8-9AD1-C57EA4339694.dmp /Users/cltbld/tasks/task_1556842754/build/symbols
00:33:13 INFO - mozcrash Saved minidump as /Users/cltbld/tasks/task_1556842754/build/blobber_upload_dir/568928C2-BD8A-48B8-9AD1-C57EA4339694.dmp
00:33:13 INFO - mozcrash Saved app info as /Users/cltbld/tasks/task_1556842754/build/blobber_upload_dir/568928C2-BD8A-48B8-9AD1-C57EA4339694.extra
00:33:13 INFO - PROCESS-CRASH | devtools/shared/webconsole/test/test_cached_messages.html | application crashed [@ JSString* js::gc::GCRuntime::tryNewNurseryString<(js::AllowGC)1>(JSContext*, unsigned long, js::gc::AllocKind)]

Comment 30

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e330d90e540084087b6a9c155eabe069ec690c49
Bug 1540984: Pre-load theme header image and share across all windows. r=aswan

Comment 31

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/e330d90e5400

Comment 32

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 68.0a1 (2019-05-04), so I'm marking this bug as VERIFIED.
Thank you very much! \o/

Comment 33

[Kestrel]

I am seeing this in 67 release with the built-in Space Fantasy theme and Starry Space Fantasy theme. Mozregression points to an older regression.

Regression window (Bug 1528451):
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d15621c6f424a75f1d4be5234c420bd9ad1f7586&tochange=2bbbcf59d8f90c50e170a0a3eab4e24bbfe528ac

Fix window (this bug):
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b758384a42e27280c1b11f2a60ff2a378949583e&tochange=e330d90e540084087b6a9c155eabe069ec690c49

Comment 34

[Emilio Cobos Álvarez (:emilio)]

This apparently also broke animated themes (https://www.reddit.com/r/firefox/comments/cbwkzv/animated_themes_arent_animated_since_v68/)

Comment 35

[Kris Maglione [:kmag]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #34)

This apparently also broke animated themes (https://www.reddit.com/r/firefox/comments/cbwkzv/animated_themes_arent_animated_since_v68/)

Breaking animated themes is not part of the problem set, as far as I'm concerned. At least not until we actually handle image animations on the compositor thread.