1338874 - |make mozmill|: JavaScript error: chrome://global/content/bindings/findbar.xml, line 423: TypeError: this.browser is null

Status: RESOLVED WORKSFORME

(Thunderbird :: Search, defect)

Description

[ISHIKAWA, Chiaki]

During |make mozmill| test suite run locally (Linux 64-bit), I see the following error.

JavaScript error: chrome://global/content/bindings/findbar.xml, line 423: TypeError: this.browser is null

The test happens in the following test and I see a few other disturbing messages, and so I quote the log for the test below
entirely.

I have no idea what the following additional three messages are...

1. [13290] WARNING: There is no observer for "invalidformsubmit". One should be implemented!: file /NREF-COMM-CENTRAL/comm-central/mozilla/dom/html/HTMLFormElement.cpp, line 2013

2. [13290] WARNING: Cannot switch to prerendered document in the parent process: file /NREF-COMM-CENTRAL/comm-central/mozilla/xpfe/appshell/nsContentTreeOwner.cpp, line 443

3. JavaScript error: file:///NREF-COMM-CENTRAL/objdir-tb3/dist/bin/components/nsLoginManager.js, line 308: Error: This login already exists.

That the test passes with these JavaScript errors is very disturbing...

TIA

TEST-START | /NREF-COMM-CENTRAL/comm-central/mail/test/mozmill/newmailaccount/test-newmailaccount.js | test_restored_ap_tab_works
++DOCSHELL 0x55a6c7eccad0 == 13 [pid = 13290] [id = {87677dfb-63ad-4b34-a218-0ace455f5c7f}]
++DOMWINDOW == 39 (0x55a6c62d4d60) [pid = 13290] [serial = 39] [outer = (nil)]
++DOMWINDOW == 40 (0x55a6c6ea9a60) [pid = 13290] [serial = 40] [outer = 0x55a6c62d4d60]
[13290] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /NREF-COMM-CENTRAL/comm-central/mozilla/chrome/nsChromeRegistry.cpp, line 213
[13290] WARNING: There is no observer for "invalidformsubmit". One should be implemented!: file /NREF-COMM-CENTRAL/comm-central/mozilla/dom/html/HTMLFormElement.cpp, line 2013
++DOCSHELL 0x55a6c6ad0ba0 == 14 [pid = 13290] [id = {97f26b1e-e646-4c53-9aee-7757c70b6141}]
++DOMWINDOW == 41 (0x55a6c6ce1850) [pid = 13290] [serial = 41] [outer = (nil)]
++DOMWINDOW == 42 (0x55a6c8185660) [pid = 13290] [serial = 42] [outer = 0x55a6c6ce1850]
++DOMWINDOW == 43 (0x55a6c9ee0e40) [pid = 13290] [serial = 43] [outer = 0x55a6c6ce1850]
JavaScript error: chrome://global/content/bindings/findbar.xml, line 423: TypeError: this.browser is null
++DOCSHELL 0x55a6c7316a90 == 15 [pid = 13290] [id = {8ede9980-20fc-4fba-b3d3-e5387d747882}]
++DOMWINDOW == 44 (0x55a6c855d580) [pid = 13290] [serial = 44] [outer = (nil)]
++DOMWINDOW == 45 (0x55a6c679ae90) [pid = 13290] [serial = 45] [outer = 0x55a6c855d580]
++DOMWINDOW == 46 (0x55a6c8bcfba0) [pid = 13290] [serial = 46] [outer = 0x55a6c6805d00]
++DOMWINDOW == 47 (0x55a6c9b529d0) [pid = 13290] [serial = 47] [outer = 0x55a6c855d580]
[13290] WARNING: There is no observer for "invalidformsubmit". One should be implemented!: file /NREF-COMM-CENTRAL/comm-central/mozilla/dom/html/HTMLFormElement.cpp, line 2013
[13290] WARNING: Cannot switch to prerendered document in the parent process: file /NREF-COMM-CENTRAL/comm-central/mozilla/xpfe/appshell/nsContentTreeOwner.cpp, line 443
++DOMWINDOW == 48 (0x55a6c43333f0) [pid = 13290] [serial = 48] [outer = 0x55a6c855d580]
JavaScript error: file:///NREF-COMM-CENTRAL/objdir-tb3/dist/bin/components/nsLoginManager.js, line 308: Error: This login already exists.
++DOCSHELL 0x55a6c9654370 == 16 [pid = 13290] [id = {6986bb3b-1276-4c6b-ab13-e6ccde20a2b2}]
++DOMWINDOW == 49 (0x55a6c9644940) [pid = 13290] [serial = 49] [outer = (nil)]
++DOMWINDOW == 50 (0x55a6c94db070) [pid = 13290] [serial = 50] [outer = 0x55a6c9644940]
++DOMWINDOW == 51 (0x55a6c5c4e480) [pid = 13290] [serial = 51] [outer = 0x55a6c6805d00]
TEST-PASS | /NREF-COMM-CENTRAL/comm-central/mail/test/mozmill/newmailaccount/test-newmailaccount.js | test-newmailaccount.js::test_restored_ap_tab_works

Comment 1

[ISHIKAWA, Chiaki]

The code in question is below.



      <!-- This is necessary because the destructor isn't called when
           we are removed from a document that is not destroyed. This
           needs to be explicitly called in this case -->
      <method name="destroy">
        <body><![CDATA[
          if (this._destroyed)
            return;
          this._destroyed = true;

--->      if (this.browser.finder)
            this.browser.finder.destroy();

          this.browser = null;

          let prefsvc = this._prefsvc;
          prefsvc.removeObserver("accessibility.typeaheadfind",
                                 this._observer);
          prefsvc.removeObserver("accessibility.typeaheadfind.linksonly",
                                 this._observer);
          prefsvc.removeObserver("accessibility.typeaheadfind.casesensitive",
                                 this._observer);
          prefsvc.removeObserver("findbar.entireword", this._observer);
          prefsvc.removeObserver("findbar.highlightAll", this._observer);
          prefsvc.removeObserver("findbar.modalHighlight", this._observer);

          // Clear all timers that might still be running.
          this._cancelTimers();
        ]]></body>
      </method>

I wonder if simple rewriting

      if (this.browser && this.browser.finder)
            this.browser.finder.destroy();

is OK or not. On the other hand, this.browser is null, maybe we can return immediately?
(Actually, the rest of the code seems to remove only the
 - observers, and
 - timer.

The actions seems to be idempotent, i.e., if we perform the actions
on objects twice or more times, the result would be the same (except for some
inefficieny), so not returning may look a safe option here unless someone has a
reason for not continuing here.

Ideally, I would be interested if there are cases where the observers, or timers exist when
this.browser was null when this part of the code is called, but that will be an icing on the cake...
 

BTW, I wonder what "we" in the comment refers to.  It seems that the
"destruction" is not synced with two associated "objects", one of whic
is a document, and it seems the other has to be "destroyed" explicitly
when a document is not destroyed and "the other object", "we" has been
"removed" (the association is broken?) from the document.

TIA

Comment 2

[Magnus Melin [:mkmelin]]

mozmill is gone