Constant Travis test failure with JavaScriptError: (17) TypeError: window.wrappedJSObject.fireMessageHandler is not a function

RESOLVED FIXED in 1.3 C1/1.4 S1(20dec)
(NeedInfo from)

Status

Firefox OS
Gaia::TestAgent
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gwagner, Assigned: markh, NeedInfo)

Tracking

unspecified
1.3 C1/1.4 S1(20dec)
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
This seems to be a gecko regression. The regression window is 
https://hg.mozilla.org/mozilla-central/pushloghtml?startID=25830&endID=25831

The travis failure looks like:
https://travis-ci.org/mozilla-b2g/gaia/jobs/15460525

It is also reproduces locally with: 'APP=email make test-integration' in the gaia folder.

Nothing really sticks out here. Very wild guesses are 944847 or 944407?
The function in question is here: https://github.com/mozilla-b2g/gaia/search?q=fireMessageHandler&ref=cmdform
(Reporter)

Comment 1

4 years ago
Bug 935793 broke it.
Blocks: 935793
(note that all "em" are stripped out from travis output, for some reason)
Gregor, note that "fireMessageHandler" is defined in some of the mocks. Not sure what's happening...
(Reporter)

Comment 4

4 years ago
(In reply to Julien Wajsberg [:julienw] from comment #2)
> (note that all "em" are stripped out from travis output, for some reason)

The error msg is fine locally.(In reply to Julien Wajsberg [:julienw] from comment #3)
> Gregor, note that "fireMessageHandler" is defined in some of the mocks. Not
> sure what's happening...

Bisect shows that bug 935793 causes the failure.
Gregor, I meant I don't get why bug 935793 broke it here.
(Reporter)

Comment 7

4 years ago
Maybe James knows whats going on.
(Assignee)

Comment 9

4 years ago
I suspect the problem is:

https://github.com/mozilla-b2g/marionette-content-script/blob/0bd1b2334d4d312707fb6330040433ec66096f9c/index.js#L21 - this code is looking for the "old" observer names before bug 935793 renamed them.

James, any suggestions for how we can land bug 935793 and a fix for the marionette code in a coordinated fashion?
(Assignee)

Comment 10

4 years ago
hrm - that code includes:

Services.obs.addObserver(
          this, 'remote-browser-frame-show', false
        );

note the missing 'n' from the observer name.  Best I can tell, this code isn't going to be hit - so only the 'in-process-browser-or-app-frame-shown' case will be hit.

Ignoring this and fixing the apparent typo in the observer name, one option would be to change the relevant functions in this file to what I've pasted (untested) below.  This should allow the code to work in both the "old" and "new" cases (and obviously the code handling the "old" cases should then be removed after the other bug lands.)  Obviously, if it transpires that 'in-process-browser-or-app-frame-shown' really is the only case we care about, this would need tweaking.

      init: function() {
        // These notifications will be sent up until bug 935793 lands.
        Services.obs.addObserver(
          this, 'remote-browser-frame-shown', false
        );

        Services.obs.addObserver(
          this, 'in-process-browser-or-app-frame-shown', false
        );
        // These notifications will be sent after bug 935793 lands.
        Services.obs.addObserver(
          this, 'remote-browser-shown', false
        );

        Services.obs.addObserver(
          this, 'inprocess-browser-shown', false
        );
      },

      observe: function(subject, topic, data) {
        var frameLoader = subject.QueryInterface(Ci.nsIFrameLoader);
        switch (topic) {
          case 'remote-browser-shown':
          case 'inprocess-browser-shown':
            // These notifications will come when bug 935793 lands, but 
            // they will be sent for all frames - we only want ones coming
            // from a "browser or app" frame
            if (!frameLoader.ownerIsBrowserOrAppFrame) {
              return;
            }
          // fall through...
          default:
            var mm = frameLoader.messageManager;

            mm.loadFrameScript(
              dataURI,
              true
            );
        }
      }
James is in PTO right now, I'm gonna needinfo the other marionette js client peers for moving this forward.
Flags: needinfo?(mike)
Flags: needinfo?(gaye)
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #5)
> Bug 935793 backed out.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a4e2de961acc

https://hg.mozilla.org/mozilla-central/rev/a4e2de961acc
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C1/1.4 S1(20dec)
Hi Julien: it looks like :RyanVM resolved this by backing out the platform regression caused by that patch for bug 935793. This makes me think that there's no Marionette client work to be done; let me know if I'm mistaken.
Flags: needinfo?(mike)
But Bug 935793 needs to eventually land, and will need a change in the marionette client to land ;)

Please see comment 10 for more information.
Flags: needinfo?(mike)
Ah, now I understand! Sorry about that, Julien--I've opened bug 952176 to track that work (and marked it as blocking bug 935793, as you might expect).
Flags: needinfo?(mike)
You need to log in before you can comment on or make changes to this bug.