1633307 - TypeError: can't access dead object TooltipTextProvider.jsm:15:7

Status: RESOLVED FIXED

(Toolkit :: General, defect, P5)

Description

[kernp25]

Attachment: firefox_0FwQNm7kZI.png

Sometimes i see this TypeError in console: can't access dead object TooltipTextProvider.jsm:15:7

Comment 1

[kernp25]

My Browser Version: 75.0 (64-Bit)

Comment 2

[Release mgmt bot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 3

[Andrew McCreight [:mccr8]]

It sounds like this code should also return false if Cu.isDeadWrapper(tipElement) is true.

Comment 4

[kernp25]

I don't know, how this bug can be reproduced.

Comment 5

[:Gijs (he/him)]

There are no negative / userv-visible side-effects here besides the error showing up, so I don't think this is P3. Without steps to reproduce, I don't know whether ignoring dead wrappers to silence the error is the best solution; it's quite possible we shouldn't be invoking the tooltip code at all, depending on some other condition (cf. bug 1633365).

Comment 6

[kernp25]

Attachment: firefox_y5nCeK9l29.png

Comment 7

[kernp25]

It seems to happen, if TooltipTextProvider.jsm is running in the content process.
I added a conditional debugger at TooltipTextProvider.jsm:14 (condition = Cu.isDeadWrapper(tipElement) == true).

Comment 8

[:Gijs (he/him)]

(In reply to kernp25 from comment #7)

It seems to happen, if TooltipTextProvider.jsm is running in the content process.
I added a conditional debugger at TooltipTextProvider.jsm:14 (condition = Cu.isDeadWrapper(tipElement) == true).

I mean, I can't reproduce, because there are no steps to reproduce, so I also can't get a conditional breakpoint to hit.

There won't be a JS stack when this happens, because the only non-test callers are in C++ code. One is in XUL code and shouldn't run in the child process. The other is in docshell code, https://searchfox.org/mozilla-central/rev/446160560bf32ebf4cb7c4e25d7386ee22667255/docshell/base/nsDocShellTreeOwner.cpp#1285 . We're supposed to clear the node when mouseout fires, but perhaps the node is getting removed before that happens? Still, even with that analysed, I can't get the error to reproduce. :mccr8, why would the node for which we're trying to fetch tooltip content be a dead wrapper?

Comment 9

[Andrew McCreight [:mccr8]]

Chrome to content references get turned into dead wrappers when the window containing the content gets closed.

Comment 10

[:Gijs (he/him)]

(In reply to Andrew McCreight [:mccr8] from comment #9)

Chrome to content references get turned into dead wrappers when the window containing the content gets closed.

But then surely, we couldn't also think that the node is in the composed doc, that the docshell is active, and that we have a presshell? https://searchfox.org/mozilla-central/rev/446160560bf32ebf4cb7c4e25d7386ee22667255/docshell/base/nsDocShellTreeOwner.cpp#1244-1281

We also clear the node reference when the page navigates ( https://searchfox.org/mozilla-central/rev/446160560bf32ebf4cb7c4e25d7386ee22667255/docshell/base/nsDocShellTreeOwner.cpp#684 )

My only other guess is that this can happen if there's an iframe with a tooltip node and the iframe gets removed, but the parent document stays, or something? But even then I'd expect IsInComposedDoc to fail, and in any case I still cannot reproduce the error.

Any other ideas?

Comment 11

[Andrew McCreight [:mccr8]]

I don't really know when we remove a node from a document, compared to when we fire the window closed event.

Comment 12

[kernp25]

Attachment: firefox_2AMNTn8hQ4.png

Screen shot of the full window.

Comment 13

[:Gijs (he/him)]

(In reply to kernp25 from comment #12)

Created attachment 9147401 [details]
firefox_2AMNTn8hQ4.png

Screen shot of the full window.

Yeah, this all really doesn't help. There's a call from C++ that is passing this information. What we need is to understand how that is possible. The JS debugger gives no visibility into this - it's not visible in the screenshot, but I imagine the stack is completely empty besides the frame in TooltipTextProvider? Even a C++ stack on its own is likely not helpful, because what is presumably happening is:

1. we schedule a check on a node to show a tooltip after N milliseconds
2. something happens that invalidates the node
3. we pass the node from C++ to JS code.

Even a C++ stack would get us a stack for the timer firing at (3), and what we need is to understand how (2) is happening.

Do these errors appear when closing add-on popups or something? Or specific websites?

Comment 14

[kernp25]

(In reply to :Gijs (he/him) from comment #13)

Do these errors appear when closing add-on popups or something? Or specific websites?

I was on my add-on settings page (about:addons) and after i clicked the browser action icon of my add-on to load yahoo in the popup (iframe).

Comment 15

[kernp25]

(In reply to :Gijs (he/him) from comment #8)

because the only non-test callers are in C++ code.

Why is the TooltipTextProvider.jsm code not in C++?

If the callers are only in C++ code?

Comment 16

[:Gijs (he/him)]

(In reply to kernp25 from comment #15)

(In reply to :Gijs (he/him) from comment #8)

because the only non-test callers are in C++ code.

Why is the TooltipTextProvider.jsm code not in C++?

If the callers are only in C++ code?

Because Gecko is like 25 years old and not everything is perfectly organized.

(In reply to kernp25 from comment #14)

(In reply to :Gijs (he/him) from comment #13)

Do these errors appear when closing add-on popups or something? Or specific websites?

I was on my add-on settings page (about:addons) and after i clicked the browser action icon of my add-on to load yahoo in the popup (iframe).

Which add-on?

Comment 17

[kernp25]

(In reply to :Gijs (he/him) from comment #16)

Which add-on?

https://addons.mozilla.org/firefox/addon/yahoo-mail-notifier-we/

But it seems not happen every time.

In my add-on background.js:

browser.runtime.onMessage.addListener((message, sender) => {
  if (sender.url == browser.extension.getURL('popup.html')) {
    return new Promise(async resolve => {
      if (message.load) {
        script = await browser.contentScripts.register({
          "css": [{file: "/content.css"}],
          "js": [{file: "/content.js"}],
          "matches": ["https://mail.yahoo.com/mb/*"],
          "allFrames": true,
          "runAt": "document_start"
        });
        addListeners();
      }
      else if (message.unload) {
        if (script) {
          script.unregister();
          script = null;
        }
        removeListeners();
      }
      resolve();
    });
  }
});

The popup.html:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8"/>
    <link rel="stylesheet" href="popup.css"/>
  </head>
  <body>
    <iframe sandbox="allow-forms allow-same-origin allow-popups allow-popups-to-escape-sandbox"></iframe>
    <script src="popup.js"></script>
  </body>
</html>

The popup.js:

'use strict';

browser.runtime.sendMessage({
  load: true,
}).then(() => {
  document.querySelector("iframe").src = "https://mail.yahoo.com/";
});

window.addEventListener("unload", event => {
  browser.runtime.sendMessage({
    unload: true,
  });
}, true);

Comment 18

[kernp25]

Attachment: 0UxGlAlKoF.mp4

Comment 19

[kernp25]

Can you look at the video?

Comment 20

[kernp25]

It also happens with this code:

<iframe sandbox="allow-forms allow-same-origin allow-popups allow-popups-to-escape-sandbox"
        src="https://mail.yahoo.com/"></iframe>

But it was not happening when loading https://example.com/ in the iframe (maybe because it was loading in the iframe very quickly?).

Comment 21

[kernp25]

I can also reproduce the bug with this code:
setTimeout(() => document.querySelector("iframe").src = "https://example.com/", 500);

Comment 22

[kernp25]

Attachment: popup.zip

Comment 23

[kernp25]

I uploaded a test add-on to reproduce the bug.

Comment 24

[:Gijs (he/him)]

I can reproduce with the test add-on in an artifact build, but not in a compiled build. :-\

It's probably timing-related. I have ideas about how to fix this on the C++ side, but no way of verifying that they work.

Comment 25

[kernp25]

I could easily reproduce it with https://mail.yahoo.com/ (without setTimeout).

Comment 26

[:Gijs (he/him)]

(In reply to kernp25 from comment #20)

It also happens with this code:

<iframe sandbox="allow-forms allow-same-origin allow-popups allow-popups-to-escape-sandbox"
        src="https://mail.yahoo.com/"></iframe>

This doesn't work for me, because the load gets blocked by x-frame-options.

Comment 27

[:Gijs (he/him)]

OK, finally managed to repro by tweaking the example.com timeout.

Comment 28

[:Gijs (he/him)]

Attachment: Bug 1633307 - avoid sending dead wrappers to JS tooltip providers, r?mccr8

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f1e45d3ca0f6
avoid sending dead wrappers to JS tooltip providers, r=smaug

Comment 30

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/f1e45d3ca0f6