1130800 - Hundreds of errors from sdk/context-menu/core in test logs

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect, P2)

Description

[Dave Townsend [:mossop] (PTO until Aug 21st)]

From the start of the test-context-menu@2 test through to the end of the entire test suite the following message shows up many many many times.

JavaScript error: resource://extensions.modules.ef3a5cb1-2ef7-472c-901f-0e16627084b8-at-jetpack.commonjs.path/toolkit/loader.js -> resource://extensions.modules.ef3a5cb1-2ef7-472c-901f-0e16627084b8-at-jetpack.commonjs.path/sdk/context-menu/core.js, line 81: TypeError: target.messageManager is undefined

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

Irakli maybe you have some idea what the issue here is?

Comment 3

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

That is really odd for few reasons:

1. The line it refers to as message handler so target there should have a messageManager associated with it should not it Dave ? It could be that my assumption what the message listeners is incorrect:
https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.js#L81

2. Also message that message listener listens to a sdk specific message names so it's unclear how why it's invoked even after tests are finished.
https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.js#L66-L67

Dave could it be that messages get queued up somehow and drained later somehow ? still not clear how `target.messageManager` would be undefined though.

I'll check to see if tests clean up properly that may also has something to do with this, but still would not explain above questions

Comment 4

[Dave Townsend [:mossop] (PTO until Aug 21st)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #3)
> That is really odd for few reasons:
> 
> 1. The line it refers to as message handler so target there should have a
> messageManager associated with it should not it Dave ? It could be that my
> assumption what the message listeners is incorrect:
> https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.
> js#L81

Possibly this is a tab that opened and closed very quickly causing the message to be received after the browser the frame is in went away, in that case target would be a dead browser element and so messageManager might be undefined in that case.

> 2. Also message that message listener listens to a sdk specific message
> names so it's unclear how why it's invoked even after tests are finished.
> https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.
> js#L66-L67

You add a delayed load framescript here: https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.js#L65

That script will run for every new tab that opens afterwards.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Dave Townsend [:mossop] from comment #4)
> (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment
> #3)
> > That is really odd for few reasons:
> > 
> > 1. The line it refers to as message handler so target there should have a
> > messageManager associated with it should not it Dave ? It could be that my
> > assumption what the message listeners is incorrect:
> > https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.
> > js#L81
> 
> Possibly this is a tab that opened and closed very quickly causing the
> message to be received after the browser the frame is in went away, in that
> case target would be a dead browser element and so messageManager might be
> undefined in that case.
> 
> > 2. Also message that message listener listens to a sdk specific message
> > names so it's unclear how why it's invoked even after tests are finished.
> > https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.
> > js#L66-L67
> 
> You add a delayed load framescript here:
> https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/context-menu/core.
> js#L65
> 
> That script will run for every new tab that opens afterwards.

So should we test if the `message.target` is not undefined to make sure it's still alive ?

Comment 6

[Dave Townsend [:mossop] (PTO until Aug 21st)]

message.target is still defined otherwise you'd be seeing different errors but if the XBL for <browser> has gone away then message.target.messageManager is undefined so you could just check that, since that is what the error complains about.

Comment 7

[Dave Townsend [:mossop] (PTO until Aug 21st)]

Actually I just realised what might be a more likely explanation. <iframe remote="true"> elements don't have messageManager properties at all. You might be hitting that.

You can do target.QueryInterface(Ci.nsIFrameLoaderOwner).frameLoader.messageManager to get the message manager of any element.

Comment 8

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562