Some themes are causing Nightly to fail. There is no crash report being generated, but tabs will fail to load content. It is not all themes: This theme will install fine (and the page is viewable): https://addons.mozilla.org/en-GB/firefox/addon/nightly-on-linux/ The theme "Nightly" by Ken Saunders will cause the active browsing pane to turn white and other tabs to continuously load. (These two are quoted as examples to replicate the issue. Other themes will cause the same symptoms.) This issue has been identified in Ubuntu 16.10 using: Mozilla/5.0 (X11; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0 ID:20170306110339 CSet: 966464a68a2cb3ca1125808e34abb5c1d34e3797 This has been verified by another user who is using BlackBox Linux. This issue has found not to be in Nightly 54 on Windows or MacOS. This issue has been found to be present after removing and reinstalling a Mozilla build of Nightly.

Thanks for reporting this Seburo. Can you use mozregression to find when this started failing? See http://mozilla.github.io/mozregression/ for more information.

Hi Jared First time using mozregression, but I believe that this is the info that you are looking for: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e91de6fb2b3dce9c932428265b0fdb630ea470d7&tochange=180a160ae22a4d63867cfd02608e7621e8e697d1 I hope this helps.

Probably due to: Jared Wein — Bug 864562 - Declare lwtheme CSS in tabs.inc.css since we don't need to iterate rules anymore. r=MattN Jared Wein — Bug 864562 - Use CSS variables for lightweight theme styling. r=MattN

Since this is specifically LWT switching, I'm thinking bug 864562 as well. However, bug 1330349 can also be the culprit. Since the browser becomes unusable, it should be caused by a JS error and easy to find/ fix.

I looked closer at this. The particular theme has '#' set as the accent color, which isn't a valid CSS color. This ends up causing the background-color to be transparent. I'm not sure why this is breaking the content processes on Linux though (they become the e10s tab throbber-spinner until the theme is changed through about:addons). We could do better validation within LightweightThemeConsumer.jsm but this seems like it would be covering up some larger problem. @billm, do you know who on the e10s team we could forward this to? It is easily reproducible when enabling the "Nightly" theme on AMO (https://addons.mozilla.org/EN-us/mobile/addon/nightly/).

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5) > when enabling the "Nightly" theme on AMO > (https://addons.mozilla.org/EN-us/mobile/addon/nightly/). The above link is wrong, it should be https://addons.mozilla.org/en-US/firefox/addon/nightly/

Mike is probably the best person to ask.

Hi For the purposes of your investigation, another example of a theme that causes this issue is "little flowers" by bluszcz (the second most popular theme listed).

Tracking 54/55+ since according to Content 0 some content is not loading.

Hey folks, happy to investigate, but I think I need a little help reproducing this one. I don't yet have enough information to make this happen on my side. What exactly do I need to do in order to see this bug?

On a clean profile, using Firefox Nightly on Linux, go to https://addons.mozilla.org/en-US/firefox/addon/nightly/ and choose "Add to Firefox". The tab will then go blank. This doesn't reproduce on Windows or OSX. The code that loads and applies lightweight themes is LightweightThemeConsumer.jsm's _update method. Rules are now implemented via CSS variables, which are defined in /browser/base/content/browser.css. See variables that start with "--lwt", http://searchfox.org/mozilla-central/search?q=--lwt-&path=

Just wanted to confirm (I did local builds) that bug 864562 is the regressing bug here. Not 100% sure what is going on here - I suspect the problem is kinda a deep one (I believe the layer manager is getting confused somehow...) Still looking.

This was a tricky one, but here's what I think is going on: 1) LightweightThemeManager.jsm sets --lwt-accent-color to "#", (which is not a valid colour value) 2) LightweightThemeManager.jsm then sets lwtheme to true 3) We do a style flush, and our style calculation infrastructure somehow decides that the "#" value means that the widget (the window) needs to become transparent. (This is the part I'm fuzziest on, but it goes through [1] at least) 4) The GTK nsWindow implementation is told to set the window to be transparent. 5) The GTK nsWindow implementation then decides to blow away the LayerManager for the window[2] My mental model about what LayerManagers do and their lifetime characteristics is not clear to me. However, I suspect that remote <xul:browser>'s are not prepared for the LayerManager to suddenly be gone like that. If we don't destroy the LayerManager, all is well. So the wall-paper fix is to prevent LightweightThemeManager.jsm from putting a non-colour into any of those CSS variables. The more general fix is probably to not blow away that LayerManager - or if so, to tell the content process about it so that it can react properly. Hey dvander, billm told me that you have a solid conception of how LayerManager's factor into e10s and how it all kinda... works. Is the above plausible? Am I right that destroying a LayerManager like this is unexpected from the content process's point of view? [1]: http://searchfox.org/mozilla-central/rev/571c1fd0ba0617f83175ccc06ed6f3eb0a1a8b92/layout/generic/nsContainerFrame.cpp#694 [2]: http://searchfox.org/mozilla-central/rev/571c1fd0ba0617f83175ccc06ed6f3eb0a1a8b92/widget/gtk/nsWindow.cpp#4318-4320 which calls http://searchfox.org/mozilla-central/rev/571c1fd0ba0617f83175ccc06ed6f3eb0a1a8b92/widget/gtk/nsWindow.cpp#4273-4288

Wow, that code predates e10s by a long time. It looks like the original bug (bug 630346) had to do with accelerated main-thread layers, which no longer exist. I would just remove it. Worst case, if the bug is still somehow reproducible, it would be much better to just invalidate layers instead, or to only blow away the layer manager if it's not a ClientLayerManager.

Hey marco, I know it's been a while, but should we just go ahead and remove the call to CleanLayerManagerRecursive? Or is it still required?

It's been a while since I worked on that patch and I haven't touched anything else related to layers acceleration since then. Karl can probably answer better than me.

Interestingly, this also breaks the browser on Windows (Win 7 when I tested this) when using a non-Aero theme (like Windows 7 Basic). Looking into it now...

So it's basically the same dealio here with the style engine calculating that the background of the :root (nsWindow) should be transparent. In classic mode, we're setting the window transparency to eTransparencyTransparent in the windows implementation of nsWindow::SetTransparencyMode, and that's causing the browser to not paint it anymore, I guess? Strange that this doesn't impact us when we're using Aero. jimm, any idea why that would be here? STR for Windows 7 would be (warning, these STR will put your browser into a broken state that survives restarts): 1) Personalize the Desktop, and switch to Windows Basic or Windows Classic themes. 2) Go to https://addons.mozilla.org/en-US/firefox/addon/nightly/ 3) Click on Add to Firefox 4) Notice how your browser is just unusable now. In order to recover, you need to enter Safe Mode and disable the LWT.

With GTK widgets at least, the layer manager type may depend on the transparency of the window. OnExposeEvent() would once generate a new layer manager of the correct type when drawing was required, but since OMTC that function may not be called until window resize, or iconify/deiconify, etc. I don't know the best way to trigger reconstruction of the layer manager with OMTC. Maybe GetLayerManager() would work, but a graphics person would need to confirm. Performance will be terrible with a change in transparency on a browser window, but it is something that should work for popup windows.

Looks like Cocoa already restricts to just popup windows. Are the warnings enough? Or should I be doing more here?

Comment on attachment 8849810 [details] Bug 1344839 - Don't allow transparent top-level windows on Windows. https://reviewboard.mozilla.org/r/122568/#review125032 ::: widget/windows/nsWindow.cpp:3042 (Diff revision 1) > void nsWindow::SetTransparencyMode(nsTransparencyMode aMode) > { > - GetTopLevelWindow(true)->SetWindowTranslucencyInner(aMode); > + nsWindow* window = GetTopLevelWindow(true); > + MOZ_ASSERT(window); > + > + if (!window) { lets add an IsDestroyed() check in here or at the top of SetWindowTranslucencyInner, so we don't drop into that code post tear down.

(In reply to Jim Mathies [:jimm] from comment #23) > Comment on attachment 8849810 [details] > Bug 1344839 - Don't allow transparent top-level windows on Windows. Wait, from the name of your commit, this sounds like it will break our desktop notification windows on Windows which use transparency. r- from me if that is the case...

Comment on attachment 8849809 [details] Bug 1344839 - Don't allow transparent top-level windows on GTK. https://reviewboard.mozilla.org/r/122566/#review125190 (In reply to Mike Conley (:mconley) (Catching up on reviews and needinfos) from comment #22) > Looks like Cocoa already restricts to just popup windows. > > Are the warnings enough? Or should I be doing more here? I don't know what will be providing an opaque background color, and so the effect might be that there is a dark background. But, thanks for pointing out Cocoa, because that means it should be fine to do something similar here. r+ with the changes noted in the issue. ::: widget/gtk/nsWindow.cpp:4293 (Diff revision 1) > + if (mIsTopLevel) { > + NS_WARNING("Cannot set transparency mode on top-level windows."); > + return; > + } > + > if (!mShell) { > // Pass the request to the toplevel window > GtkWidget *topWidget = GetToplevelWidget(); > if (!topWidget) > return; > > nsWindow *topWindow = get_window_for_gtk_widget(topWidget); > if (!topWindow) > return; > > topWindow->SetTransparencyMode(aMode); > return; > } s/mIsTopLevel/mWindowType != eWindowType_popup/ because mIsTopLevel means not-child on this system (and case is important in GetToplevelWidget() :/). Also, please move the new code to after the !mShell block, so that the check is performed on the toplevel (not child) window.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #24) > (In reply to Jim Mathies [:jimm] from comment #23) > > Comment on attachment 8849810 [details] > > Bug 1344839 - Don't allow transparent top-level windows on Windows. > > Wait, from the name of your commit, this sounds like it will break our > desktop notification windows on Windows which use transparency. r- from me > if that is the case... I just tested the patches on this bug and they do *not* break the transparency for desktop notifications. Thanks for your work here! :)

Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/aaaf793a4dd0 Don't allow transparent top-level windows on GTK. r=karlt https://hg.mozilla.org/integration/autoland/rev/b6a97b374a6d Don't allow transparent top-level windows on Windows. r=jimm

https://hg.mozilla.org/mozilla-central/rev/aaaf793a4dd0 https://hg.mozilla.org/mozilla-central/rev/b6a97b374a6d

Backed out for permafailing test_leaf_layers_partition_browser_window.xul on Windows 8 x64 debug: https://hg.mozilla.org/integration/autoland/rev/b667afa177438a264237894779804946d2a8e7bc https://hg.mozilla.org/integration/autoland/rev/22868f3a400d955848a268e46f0caccdc899ec24 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=b6a97b374a6d1349690b4b92bcb97baafd75e18c&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=87050635&repo=autoland 11:39:16 INFO - GECKO(3380) | [3380] WARNING: '!aObserver', file c:/builds/moz2_slave/autoland-w64-d-000000000000000/build/src/xpcom/ds/nsObserverService.cpp, line 234 11:39:16 INFO - GECKO(3380) | nsStringStats 11:39:16 INFO - GECKO(3380) | => mAllocCount: 146 11:39:16 INFO - GECKO(3380) | => mReallocCount: 1 11:39:16 INFO - GECKO(3380) | => mFreeCount: 146 11:39:16 INFO - GECKO(3380) | => mShareCount: 327 11:39:16 INFO - GECKO(3380) | => mAdoptCount: 0 11:39:16 INFO - GECKO(3380) | => mAdoptFreeCount: 0 11:39:16 INFO - GECKO(3380) | => Process ID: 3092, Thread ID: 2508 11:40:34 INFO - GECKO(3380) | [3380] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file c:/builds/moz2_slave/autoland-w64-d-000000000000000/build/src/widget/windows/WinUtils.cpp, line 1468 11:42:34 INFO - GECKO(3380) | [3380] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80520012: file c:/builds/moz2_slave/autoland-w64-d-000000000000000/build/src/widget/windows/WinUtils.cpp, line 1468 11:44:08 INFO - TEST-INFO | started process screenshot 11:44:08 INFO - TEST-INFO | screenshot: exit 0 11:44:08 INFO - Buffered messages logged at 11:38:49 11:44:08 INFO - TEST-PASS | layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | mozPaintCount has increased (non-maximized) 11:44:08 INFO - Buffered messages finished 11:44:08 INFO - TEST-UNEXPECTED-FAIL | layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | Test timed out. 11:44:08 INFO - reportError@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:120:7 11:44:08 INFO - TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:141:7 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:162:5 11:44:08 INFO - TestRunner.runTests@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:379:5 11:44:08 INFO - RunSet.runtests@chrome://mochikit/content/tests/SimpleTest/setup.js:190:3 11:44:08 INFO - RunSet.runall@chrome://mochikit/content/tests/SimpleTest/setup.js:169:5 11:44:08 INFO - hookupTests@chrome://mochikit/content/tests/SimpleTest/setup.js:262:5 11:44:08 INFO - parseTestManifest@chrome://mochikit/content/manifestLibrary.js:38:5 11:44:08 INFO - getTestManifest/req.onload@chrome://mochikit/content/manifestLibrary.js:49:11 11:44:08 INFO - EventHandlerNonNull*getTestManifest@chrome://mochikit/content/manifestLibrary.js:45:3 11:44:08 INFO - hookup@chrome://mochikit/content/tests/SimpleTest/setup.js:242:5 11:44:08 INFO - linkAndHookup@chrome://mochikit/content/harness.xul:59:3 11:44:08 INFO - parseTestManifest@chrome://mochikit/content/manifestLibrary.js:38:5 11:44:08 INFO - getTestManifest/req.onload@chrome://mochikit/content/manifestLibrary.js:49:11 11:44:08 INFO - EventHandlerNonNull*getTestManifest@chrome://mochikit/content/manifestLibrary.js:45:3 11:44:08 INFO - getTestList@chrome://mochikit/content/chrome-harness.js:260:3 11:44:08 INFO - loadTests@chrome://mochikit/content/harness.xul:38:3 11:44:08 INFO - EventListener.handleEvent*@chrome://mochikit/content/harness.xul:62:5

Merge of backout: https://hg.mozilla.org/mozilla-central/rev/b667afa17743 https://hg.mozilla.org/mozilla-central/rev/22868f3a400d

It broke the UI of the last Nightly on Windows 7, I dont have anymore minimize/close button and the transparency of tabs is weird. Could you rebuild Nighlty?

Done!

(In reply to Ryan VanderMeulen [:RyanVM] from comment #36) > Done! Ty! :)

as a note there are some talos changes with this- here is the list of change when we backed this out: == Change summary for alert #5725 (as of March 28 2017 16:51 UTC) == Regressions: 5% tresize windows7-32 opt 11.15 -> 11.75 Improvements: 8% tscrollx summary windows7-32 opt 2.8 -> 2.57 8% glterrain summary windows7-32 opt 5.24 -> 4.83 6% tart summary windows7-32 opt 7.09 -> 6.7 4% tp5o % Processor Time windows7-32 opt 90.33 -> 86.87 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=5725 that means that we had a tresize improvement and regressions in glterrain, tscrollx, tart, and tp5o processor time. All on win7 opt (not e10s).

Comment on attachment 8849810 [details] Bug 1344839 - Don't allow transparent top-level windows on Windows. re-requesting review.

Try looks green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=39461bdf3197b71d325758b0d930832be066c5c6

(In reply to Loic from comment #35) > It broke the UI of the last Nightly on Windows 7, I dont have anymore > minimize/close button and the transparency of tabs is weird. This was caught on mozscreenshots for Windows 8 too: https://screenshots.mattn.ca/compare/?oldProject=mozilla-central&oldRev=0e0eb96528a1d032fe6ed54f67d32290d533fbfd&newProject=mozilla-central&newRev=e23cf1b38ad4b55416318d205864195d3666b4f3

Comment on attachment 8849810 [details] Bug 1344839 - Don't allow transparent top-level windows on Windows. https://reviewboard.mozilla.org/r/122568/#review129528

Pushed by mconley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6ad9dd231334 Don't allow transparent top-level windows on GTK. r=karlt https://hg.mozilla.org/integration/autoland/rev/9e3e55b1d8d6 Don't allow transparent top-level windows on Windows. r=jimm

https://hg.mozilla.org/mozilla-central/rev/6ad9dd231334 https://hg.mozilla.org/mozilla-central/rev/9e3e55b1d8d6

Comment on attachment 8849809 [details] Bug 1344839 - Don't allow transparent top-level windows on GTK. Approval Request Comment [Feature/Bug causing the regression]: Bug 864562 exposed it. [User impact if declined]: There are a few lightweight themes that we make available that, depending on the user's OS and configuration, might put the browser into a broken state (can't see web content, and sluggish tab switching on Linux, unresponsive window on Windows if DWM composition is disabled). [Is this code covered by automated tests?]: Yes, I'd say so - though this particular bug does not have a regression test for it. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: Couldn't hurt. STR (Linux) are in comment 11. STR (Windows) are in comment 18. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: This is pretty self-contained - we're disallowing window transparency for top-level browser windows on Linux and Windows under certain conditions. We've been doing the same for OS X for a number of years now. [String changes made/needed]: None.

Comment on attachment 8849810 [details] Bug 1344839 - Don't allow transparent top-level windows on Windows. See above.

Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!

I reproduced the issue using Nightly 54.0a1 (2017-03-06) build on Ubuntu 16.04 x64 (using the STR from comment 11) and on Windows 7 x64(using the STR from comment 18). Verified as fixed using the latest Nightly 55.0a1 (2017-04-11) on Ubuntu 16.04 x64 and Windows 7 x64(using Windows Basic and Windows Classic themes).

Comment on attachment 8849809 [details] Bug 1344839 - Don't allow transparent top-level windows on GTK. Fix a regression and was verified. Aurora54+.

https://hg.mozilla.org/releases/mozilla-aurora/rev/49d5a4f14a07 https://hg.mozilla.org/releases/mozilla-aurora/rev/e261ddc45da9

I've reproduced this issue using STR from comment 0 with an old Nightly build. I can no longer reproduce this on 54 beta 2 (20170424145525) under Windows 7 x64 and Ubuntu 16.04 x64 LTS, following the STR from comment 49.