Enable Firefox Monitor by default
Categories
(Firefox :: Firefox Monitor, enhancement)
Tracking
()
People
(Reporter: nhnt11, Assigned: nhnt11)
References
Details
Attachments
(5 files)
We've been using a Normandy recipe to flip the pref and enable the add-on for 100% of release users.
Now that this is in the tree, we should pref it on.
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Comment 2•5 years ago
|
||
Johann, I've been postponing this preffing-on patch because we still haven't gotten data-review. Unfortunately it's been a bit difficult to sync with Sandy on the data-review request doc and get it checked out.
I have a meeting scheduled with her today though, and hopefully we can get data-review+ in the next couple of days. We'll have to uplift the patch to enable Telemetry - we should land this along with that and uplift them to beta together.
I prefer not to do uplifts like this, but I think it's unavoidable in this case. I think the risk is minimal since this has been QA'd and we have been shipping on Release channel via Go Faster and Normandy for a few releases now.
As the standing peer reviewer for Firefox Monitor code, I wanted to make sure you had a chance to comment on the situation. Thanks!
|
||
Comment 3•5 years ago
|
||
Yeah, considering that 100% of release has this, I think the more dangerous course of action is actually to leave it preffed off in Beta and Nightly, thus getting less exposure with the audience that actually files bugs. The main idea of the pre-release channels is to protect release users from regressions and there's no risk in that here.
Thanks for checking, though!
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/7d5b25ce9e8f Enable Firefox Monitor by default. r=johannh
Backout by rgurzau@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bd4178c6c695 Backed out changeset 7d5b25ce9e8f for osx mochitest failures on a CLOSED TREE.
|
||
Comment 6•5 years ago
|
||
Backed out changeset 7d5b25ce9e8f (bug 1531838)for osx mochitest failures on a CLOSED TREE.
Backout link: https://hg.mozilla.org/integration/autoland/rev/bd4178c6c6950034712acf97637cf73326139a62
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=7d5b25ce9e8f12276156ac98b7e46a0c71863a49&selectedJob=236799633
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=236799633&repo=autoland&lineNumber=2251
Log snippet:
[task 2019-03-29T03:08:36.635Z] 03:08:36 INFO - Entering test bound test1b
[task 2019-03-29T03:08:36.635Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | Test 1b, Should have a click-to-play notification -
[task 2019-03-29T03:08:36.635Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | Test 1b, Plugin should not be activated - true == true -
[task 2019-03-29T03:08:36.635Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | 'Remember' checkbox should be hidden in private windows -
[task 2019-03-29T03:08:36.636Z] 03:08:36 INFO - Leaving test bound test1b
[task 2019-03-29T03:08:36.636Z] 03:08:36 INFO - Entering test bound test2a
[task 2019-03-29T03:08:36.636Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | Test 2a, Should have a click-to-play notification -
[task 2019-03-29T03:08:36.636Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | Test 2a, Plugin should not be activated - true == true -
[task 2019-03-29T03:08:36.636Z] 03:08:36 INFO - Leaving test bound test2a
[task 2019-03-29T03:08:36.637Z] 03:08:36 INFO - Entering test bound test2c
[task 2019-03-29T03:08:36.637Z] 03:08:36 INFO - Console message: [JavaScript Warning: "Array.forEach is deprecated; use Array.prototype.forEach instead" {file: "chrome://global/content/bindings/tabbox.xml" line: 162}]
[task 2019-03-29T03:08:36.638Z] 03:08:36 INFO - Console message: [JavaScript Warning: "Array.unshift is deprecated; use Array.prototype.unshift instead" {file: "chrome://browser/content/tabbrowser.js" line: 4821}]
[task 2019-03-29T03:08:36.638Z] 03:08:36 INFO - Console message: [JavaScript Warning: "Array.filter is deprecated; use Array.prototype.filter instead" {file: "chrome://browser/content/tabbrowser.js" line: 239}]
[task 2019-03-29T03:08:36.638Z] 03:08:36 INFO - Buffered messages logged at 03:08:36
[task 2019-03-29T03:08:36.639Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | should have created a private window. -
[task 2019-03-29T03:08:36.639Z] 03:08:36 INFO - loaded http://127.0.0.1:8888/browser/browser/base/content/test/plugins/plugin_test.html
[task 2019-03-29T03:08:36.641Z] 03:08:36 INFO - Buffered messages finished
[task 2019-03-29T03:08:36.642Z] 03:08:36 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/plugins/browser_private_clicktoplay.js | Test 2c, Should have a click-to-play notification -
[task 2019-03-29T03:08:36.642Z] 03:08:36 INFO - Stack trace:
[task 2019-03-29T03:08:36.642Z] 03:08:36 INFO - chrome://mochikit/content/browser-test.js:test_ok:1304
[task 2019-03-29T03:08:36.642Z] 03:08:36 INFO - chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_private_clicktoplay.js:test2c:114
[task 2019-03-29T03:08:36.642Z] 03:08:36 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1106
[task 2019-03-29T03:08:36.643Z] 03:08:36 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1134
[task 2019-03-29T03:08:36.646Z] 03:08:36 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:995
[task 2019-03-29T03:08:36.646Z] 03:08:36 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-03-29T03:08:36.646Z] 03:08:36 INFO - TEST-PASS | browser/base/content/test/plugins/browser_private_clicktoplay.js | Test 2c, Plugin should be activated - true == true -
[task 2019-03-29T03:08:36.647Z] 03:08:36 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-03-29T03:08:36.647Z] 03:08:36 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/plugins/browser_private_clicktoplay.js | Uncaught exception - at chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_private_clicktoplay.js:125 - TypeError: popupNotification is null
[task 2019-03-29T03:08:36.648Z] 03:08:36 INFO - Stack trace:
[task 2019-03-29T03:08:36.648Z] 03:08:36 INFO - test2c@chrome://mochitests/content/browser/browser/base/content/test/plugins/browser_private_clicktoplay.js:125:3
[task 2019-03-29T03:08:36.648Z] 03:08:36 INFO - AsyncTester_execTest/<@chrome://mochikit/content/browser-test.js:1106:34
[task 2019-03-29T03:08:36.648Z] 03:08:36 INFO - asyncTester_execTest@chrome://mochikit/content/browser-test.js:1134:12
[task 2019-03-29T03:08:36.649Z] 03:08:36 INFO - nextTest/<@chrome://mochikit/content/browser-test.js:995:14
[task 2019-03-29T03:08:36.649Z] 03:08:36 INFO - SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:803:59
[task 2019-03-29T03:08:36.649Z] 03:08:36 INFO - Leaving test bound test2c
[task 2019-03-29T03:08:36.650Z] 03:08:36 INFO - Entering test bound test3a
|
|
Assignee | |
Comment 7•5 years ago
•
|
||
This is a funky failure. I noticed some other failures related to leaving properties behind on windows (because fxmonitor dynamically sets two properties on every window, and since it happens after delayed startup it seems to break that check). To mitigate this, I inlined the PanelUI code into FirefoxMonitor.jsm, and replaced these window properties with Maps in the FirefoxMonitor object that track the same information per-window. This not only seems to fix the properties-left-behind failures but also the clicktoplay test mentioned in the comment above.
Try push is here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1306ab07e5fa216bea1a81fdb3be69c97813394
https://treeherder.mozilla.org/#/jobs?repo=try&revision=acbf632e288e2a8c29a24095da6ea6cb07d36590
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2f56f734817f88577372307acf8ae6c19b08c7f
I'll request review on the new patch once the results are in, if it's green.
|
|
Assignee | |
Comment 8•5 years ago
•
|
||
Had to also use TestUtils.waitForCondition to reliably wait for the click-to-play notification in private windows in browser_private_clicktoplay.js. This should solve all the issues.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac9cc886ad2728cb3a269117a37764e96b0471d5
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5901e9360a28c8937bd194b81e38a9ccb8fc9df1
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 9•5 years ago
|
||
Depends on D23837
|
|
Assignee | |
Comment 10•5 years ago
|
||
Depends on D26699
|
|
Assignee | |
Comment 11•5 years ago
|
||
Depends on D26700
|
||
Comment 12•5 years ago
|
||
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/712bd9022cc2 Enable Firefox Monitor by default. r=johannh https://hg.mozilla.org/integration/autoland/rev/f54fe4e4822f Inline PanelUI code and avoid leaving behind properties on windows. r=johannh https://hg.mozilla.org/integration/autoland/rev/1badfbad01fb Reliably wait for click-to-play popup notification in private windows. r=johannh https://hg.mozilla.org/integration/autoland/rev/5c20b75a9e32 Ensure we don't set up twice on any window. r=johannh
|
||
Comment 13•5 years ago
|
||
Backed out 4 changesets (Bug 1531838) for assertion failures on ClientLayerManager.cpp
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=5c20b75a9e32bbd62d1709eb42d3323ddfbe1d28&selectedJob=239079807
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=239079807&repo=autoland&lineNumber=3384
Backout: https://hg.mozilla.org/integration/autoland/rev/917d4b516ba958539d8145c13fab63a5344e953d
|
|
Assignee | |
Comment 14•5 years ago
|
||
Markus, this bug is trying to enable a system add-on that watches windows, and injects a PopupNotification UI into each one that is displayed upon certain triggers.
It seems like the patches are causing an assertion failure in ClientLayerManager.cpp - https://searchfox.org/mozilla-central/rev/8d78f219702286c873860f39f9ed78bad1a6d062/gfx/layers/client/ClientLayerManager.cpp#84
You added this assertion - could you help me understand exactly what it's trying to catch so I can figure out what's causing it to fail?
Please let me know what questions I can answer to help you understand the situation and also to redirect the ni? if someone else is better equipped to help out with this.
|
||
Comment 15•5 years ago
|
||
Uh oh, looks like you've found out how to reliably trigger an existing issue.
Matt, could you look into fixing this? Looks like a window's layer manager is being destroyed in a reentrant way, because the call to nsView::DidCompositeWindow somehow causes a stylesheet to be loaded synchronously, which spins a nested event loop... Unfortunately there's no complete stack in the log.
Nihanth, are you using MozAfterPaint listeners in this code somewhere? Are you opening a window from one? If so, you might be able to work around this assertion failure by opening the window asynchronously.
|
|
Assignee | |
Comment 16•5 years ago
•
|
||
(In reply to Markus Stange [:mstange] from comment #15)
Uh oh, looks like you've found out how to reliably trigger an existing issue.
Matt, could you look into fixing this? Looks like a window's layer manager is being destroyed in a reentrant way, because the call to
nsView::DidCompositeWindowsomehow causes a stylesheet to be loaded synchronously, which spins a nested event loop... Unfortunately there's no complete stack in the log.Nihanth, are you using MozAfterPaint listeners in this code somewhere? Are you opening a window from one? If so, you might be able to work around this assertion failure by opening the window asynchronously.
I followed up with Markus on IRC and it seems like the issue is that we are manually injecting a stylesheet in a browser-delayed-startup-finished callback, which happens in a MozAfterPaint listener. I'm going to try putting the stylesheet injection (and removal) in Services.tm.dispatchToMainThread() (to avoid spinning a nested event loop) and see if that fixes the issue.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00e65d52dbfdfbc59e2e7eea3662528c9d38cae9
|
|
Assignee | |
Comment 17•5 years ago
|
||
This avoids an AssertionError when loading a stylesheet in a nested
event loop. See comment 15 in the bug.
Depends on D26701
|
|
||
Comment 18•5 years ago
|
||
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/9b0dab8154c0 Enable Firefox Monitor by default. r=johannh https://hg.mozilla.org/integration/autoland/rev/ae727251b802 Inline PanelUI code and avoid leaving behind properties on windows. r=johannh https://hg.mozilla.org/integration/autoland/rev/3e538caf5af2 Reliably wait for click-to-play popup notification in private windows. r=johannh https://hg.mozilla.org/integration/autoland/rev/24d7fb7fd3af Ensure we don't set up twice on any window. r=johannh https://hg.mozilla.org/integration/autoland/rev/7e886806ae98 Inject/remove stylesheets in a Services.tm.dispatchToMainThread call. r=johannh
|
||
Comment 19•5 years ago
|
||
Backed out for causing linux asan leaks
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=239778106&repo=autoland&lineNumber=1769
Backout: https://hg.mozilla.org/integration/autoland/rev/236d2bc601b0604b30e03b2e61a50a3886961edb
[task 2019-04-11T22:18:20.442Z] 22:18:20 INFO - TEST-START | devtools/client/framework/test/browser_browser_toolbox.js
[task 2019-04-11T22:18:40.246Z] 22:18:40 ERROR - GECKO(1396) | ==1606==ERROR: LeakSanitizer: detected memory leaks
[task 2019-04-11T22:18:40.256Z] 22:18:40 INFO - GECKO(1396) | Indirect leak of 32768 byte(s) in 1 object(s) allocated from:
[task 2019-04-11T22:18:40.259Z] 22:18:40 INFO - GECKO(1396) | #0 0x55780d9c5503 in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
[task 2019-04-11T22:18:40.260Z] 22:18:40 INFO - GECKO(1396) | #1 0x7ff8c52c4766 in nsSegmentedBuffer::AppendNewSegment() /builds/worker/workspace/build/src/xpcom/io/nsSegmentedBuffer.cpp:55:22
[task 2019-04-11T22:18:40.261Z] 22:18:40 INFO - GECKO(1396) | #2 0x7ff8c52c3f19 in nsPipe::GetWriteSegment(char*&, unsigned int&) /builds/worker/workspace/build/src/xpcom/io/nsPipe3.cpp:813:25
[task 2019-04-11T22:18:40.262Z] 22:18:40 INFO - GECKO(1396) | #3 0x7ff8c52cd8a2 in nsPipeOutputStream::WriteSegments(nsresult ()(nsIOutputStream, void*, char*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) /builds/worker/workspace/build/src/xpcom/io/nsPipe3.cpp:1641:17
[task 2019-04-11T22:18:40.263Z] 22:18:40 INFO - GECKO(1396) | #4 0x7ff8c539c091 in NS_InvokeByIndex /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcinvoke_asm_x86_64_unix.S:106
[task 2019-04-11T22:18:40.264Z] 22:18:40 INFO - GECKO(1396) | #5 0x7ff8c6efa71f in Invoke /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1624:10
[task 2019-04-11T22:18:40.264Z] 22:18:40 INFO - GECKO(1396) | #6 0x7ff8c6efa71f in Call /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1171
[task 2019-04-11T22:18:40.265Z] 22:18:40 INFO - GECKO(1396) | #7 0x7ff8c6efa71f in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNative.cpp:1137
[task 2019-04-11T22:18:40.265Z] 22:18:40 INFO - GECKO(1396) | #8 0x7ff8c6f0082d in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:942:10
[task 2019-04-11T22:18:40.266Z] 22:18:40 INFO - GECKO(1396) | #9 0x7ff8d158b130 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
[task 2019-04-11T22:18:40.267Z] 22:18:40 INFO - GECKO(1396) | #10 0x7ff8d158b130 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
[task 2019-04-11T22:18:40.267Z] 22:18:40 INFO - GECKO(1396) | #11 0x7ff8d156c742 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
[task 2019-04-11T22:18:40.268Z] 22:18:40 INFO - GECKO(1396) | #12 0x7ff8d156c742 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3072
[task 2019-04-11T22:18:40.269Z] 22:18:40 INFO - GECKO(1396) | #13 0x7ff8d1556c08 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
[task 2019-04-11T22:18:40.269Z] 22:18:40 INFO - GECKO(1396) | #14 0x7ff8d158baa3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:562:13
[task 2019-04-11T22:18:40.269Z] 22:18:40 INFO - GECKO(1396) | #15 0x7ff8d158d722 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:605:8
[task 2019-04-11T22:18:40.270Z] 22:18:40 INFO - GECKO(1396) | #16 0x7ff8d1aa3c9d in js::fun_apply(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/js/src/vm/JSFunction.cpp:1183:10
[task 2019-04-11T22:18:40.271Z] 22:18:40 INFO - GECKO(1396) | #17 0x7ff8d158b130 in CallJSNative /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:442:13
[task 2019-04-11T22:18:40.271Z] 22:18:40 INFO - GECKO(1396) | #18 0x7ff8d158b130 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:534
[task 2019-04-11T22:18:40.279Z] 22:18:40 INFO - GECKO(1396) | #19 0x7ff8d156c742 in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:593:10
[task 2019-04-11T22:18:40.281Z] 22:18:40 INFO - GECKO(1396) | #20 0x7ff8d156c742 in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3072
[task 2019-04-11T22:18:40.282Z] 22:18:40 INFO - GECKO(1396) | #21 0x7ff8d1556c08 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:422:10
[task 2019-04-11T22:18:40.283Z] 22:18:40 INFO - GECKO(1396) | #22 0x7ff8d158baa3 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:562:13
[task 2019-04-11T22:18:40.287Z] 22:18:40 INFO - GECKO(1396) | #23 0x7ff8d158d722 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:605:8
[task 2019-04-11T22:18:40.288Z] 22:18:40 INFO - GECKO(1396) | #24 0x7ff8d21cc047 in JS_CallFunctionValue(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2557:10
[task 2019-04-11T22:18:40.288Z] 22:18:40 INFO - GECKO(1396) | #25 0x7ff8c6ee980a in nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:965:17
[task 2019-04-11T22:18:40.289Z] 22:18:40 INFO - GECKO(1396) | #26 0x7ff8c539d78a in PrepareAndDispatch /builds/worker/workspace/build/src/xpcom/reflect/xptcall/md/unix/xptcstubs_x86_64_linux.cpp:127:37
[task 2019-04-11T22:18:40.290Z] 22:18:40 INFO - GECKO(1396) | #27 0x7ff8c539c66a in SharedStub (/builds/worker/workspace/build/application/firefox/libxul.so+0x229866a)
[task 2019-04-11T22:18:40.291Z] 22:18:40 INFO - GECKO(1396) | #28 0x7ff8c52e1d86 in nsOutputStreamReadyEvent::Run() /builds/worker/workspace/build/src/xpcom/io/nsStreamUtils.cpp:173:20
[task 2019-04-11T22:18:40.292Z] 22:18:40 INFO - GECKO(1396) | #29 0x7ff8c53712b1 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1180:14
[task 2019-04-11T22:18:40.293Z] 22:18:40 INFO - GECKO(1396) | #30 0x7ff8c53773c8 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:486:10
[task 2019-04-11T22:18:40.294Z] 22:18:40 INFO - GECKO(1396) | #31 0x7ff8c63b6a1a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:88:21
[task 2019-04-11T22:18:40.297Z] 22:18:40 INFO - GECKO(1396) | #32 0x7ff8c62e4392 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:315:10
[task 2019-04-11T22:18:40.298Z] 22:18:40 INFO - GECKO(1396) | #33 0x7ff8c62e4392 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:308
[task 2019-04-11T22:18:40.299Z] 22:18:40 INFO - GECKO(1396) | #34 0x7ff8c62e4392 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:290
[task 2019-04-11T22:18:40.299Z] 22:18:40 INFO - GECKO(1396) | #35 0x7ff8cd714e69 in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:137:27
[task 2019-04-11T22:18:40.300Z] 22:18:40 INFO - GECKO(1396) | #36 0x7ff8d108def0 in nsAppStartup::Run() /builds/worker/workspace/build/src/toolkit/components/startup/nsAppStartup.cpp:270:30
[task 2019-04-11T22:18:40.301Z] 22:18:40 INFO - GECKO(1396) | #37 0x7ff8d12fca0e in XREMain::XRE_mainRun() /builds/worker/workspace/build/src/toolkit/xre/nsAppRunner.cpp:4572:22
|
|
Assignee | |
Comment 20•5 years ago
|
||
Oof, I didn't notice this in the previous backout, and I don't know immediately what's going on... I didn't expect this to be so fragile. I'll investigate this with a clear head tomorrow.
|
|
Assignee | |
Comment 21•5 years ago
|
||
Gijs helped me analyze this on IRC. This leak seems identical to the one being tracked in bug 1444716 comment 7. Seems like the Firefox Monitor code is doing something that changes some timing somewhere that causes things to be, um, less intermittent.
|
|
Assignee | |
Comment 22•5 years ago
•
|
||
On a whim, I refactored the code that sets up the PanelUI to run lazily the first time we actually need to show a notification, rather than adding DOM elements right after delayed startup.
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=82698e70167439a16a74eeeba60d0ca2f1956230
|
|
Assignee | |
Comment 23•5 years ago
|
||
That didn't do it.
I made patches to merge all the code into api.js (i.e. no more subscripts, no more FirefoxMonitor.jsm), and to punt delayedInit() to right before we call warnIfNeeded - i.e. after we detect a page load.
This avoids doing a lot of I/O (to load subscripts, remote settings, etc) and hopefully stops this code from interfering with tests and triggering memory leaks etc.
Try-all-the-things push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3772ca5d3ae52c8bb3d525bc70dbafbdf02e38a7
|
|
Assignee | |
Comment 24•5 years ago
|
||
Seems like just making PanelUI setup be lazy DID do the trick.
Here's a try-all-the-things push just in case: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b44074e15604da31c1b5ce4ec8989daab21c7a1
|
||
Updated•5 years ago
|
|
|
||
Comment 25•5 years ago
|
||
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/56b4bb2871d1 Enable Firefox Monitor by default. r=johannh https://hg.mozilla.org/integration/autoland/rev/9f7b63f61c83 Inline PanelUI code and avoid leaving behind properties on windows. r=johannh https://hg.mozilla.org/integration/autoland/rev/d6e09cefb3c7 Reliably wait for click-to-play popup notification in private windows. r=johannh https://hg.mozilla.org/integration/autoland/rev/2058918f63f0 Ensure we don't set up twice on any window. r=johannh https://hg.mozilla.org/integration/autoland/rev/ce0b0350b646 Set up fxmonitor PanelUI only when actually showing a notification. r=johannh
|
||
Comment 26•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/56b4bb2871d1
https://hg.mozilla.org/mozilla-central/rev/9f7b63f61c83
https://hg.mozilla.org/mozilla-central/rev/d6e09cefb3c7
https://hg.mozilla.org/mozilla-central/rev/2058918f63f0
https://hg.mozilla.org/mozilla-central/rev/ce0b0350b646
|
|
Assignee | |
Comment 27•5 years ago
|
||
Comment on attachment 9051651 [details]
Bug 1531838 - Enable Firefox Monitor by default. r=johannh
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1525519
- User impact if declined: User will continue to have Firefox Monitor enabled remotely via Normandy if we don't uplift this.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: Bug 1525977
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This change simply enables Firefox Monitor by default, along with a few changes to fix test failures.
We've already been shipping this feature to the entire release population via Go Faster and Normandy - this simply changes the enabled-state in-tree so we don't have to enable it remotely.
If we don't uplift this, we'll have to continue to use Normandy to enable the feature in 67, which is not recommended by the Normandy team.
- String changes made/needed: none
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
|
Assignee | |
Updated•5 years ago
|
|
||
Comment 28•5 years ago
|
||
67 will get the addon pref via normandy.
|
||
Comment 29•5 years ago
|
||
(In reply to Markus Stange [:mstange] from comment #15)
Uh oh, looks like you've found out how to reliably trigger an existing issue.
Matt, could you look into fixing this? Looks like a window's layer manager is being destroyed in a reentrant way, because the call to
nsView::DidCompositeWindowsomehow causes a stylesheet to be loaded synchronously, which spins a nested event loop... Unfortunately there's no complete stack in the log.Nihanth, are you using MozAfterPaint listeners in this code somewhere? Are you opening a window from one? If so, you might be able to work around this assertion failure by opening the window asynchronously.
I filed bug 1548189 for this.
Description
•