1017808 - 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

Status: RESOLVED FIXED

(Firefox :: New Tab Page, defect)

Description

[David Teller [:Yoric] - still alive but not very active]

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.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

Attachment: Making browser.xml's get_messageManager more robust

Comment 2

[:Felipe Gomes (needinfo for replies!)]

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..

Comment 4

[Drew Willcoxon :adw]

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.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

So, can I land it?

Comment 6

[:Felipe Gomes (needinfo for replies!)]

See my comment in bug 1019670 comment 2. Ping me on IRC and I can help with that.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

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?

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Will do.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/fx-team/rev/3fa3c76ac61f

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3fa3c76ac61f