See https://tbpl.mozilla.org/php/getParsedLog.php?id=40625663&tree=Try&full=1#error0 This will become an actual orange once bug 1016387 lands.
Summary: TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/newtab/browser_newtab_bug752841.js | A promise chain failed to handle a rejection at chrome://global/content/bindings/browser.xml:299 - A promise chain failed to handle a → TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/newtab/browser_newtab_bug752841.js | A promise chain failed to handle a rejection at chrome://global/content/bindings/browser.xml:299
Created attachment 8433375 [details] [diff] [review] Making browser.xml's get_messageManager more robust
Assignee: nobody → dteller
Attachment #8433375 - Flags: review?(felipc)
I could give a r+ for the change itself (hard to argue against "making xx more robust"), but I wonder why this failure is happening in the first place (a browser without a frameLoader?). Checking to see if Drew has any ideas..
Duplicate of this bug: 1017813
Thanks for flagging me. This is the error I was expecting to see in bug 1013722. When I was debugging it and actually looking at browser.QI(Ci.nsIFrameLoaderOwner) and browser.QI(Ci.nsIFrameLoaderOwner).frameLoader, what I saw was that browser.QI(Ci.nsIFrameLoaderOwner).frameLoader was null, so the messageManager getter should have been throwing an error like this one when it does frameLoader.messageManager, but instead the getter was returning undefined. I fixed my bug and didn't ask questions. What I seemed to be seeing in bug 1013722 was that frameLoader was null after the browser had been closed. Given that, I think this patch here is good. Maybe the alternative is to find out why it's null in that case and if anything can be done about it.
So, can I land it?
See my comment in bug 1019670 comment 2. Ping me on IRC and I can help with that.
Comment on attachment 8433375 [details] [diff] [review] Making browser.xml's get_messageManager more robust I'm more convinced that this patch will be the right thing to do in the end, so r+. Though I think it would be good to understand what set of tests causes the problem to show up. It could be simply that this patch is missing, or that a test or the service itself is adding a message listener and forgeting to remove it. Can you file a follow up for that?
Attachment #8433375 - Flags: review?(felipc) → review+
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
You need to log in before you can comment on or make changes to this bug.