1117708 - [e10s] Using the element picker on a new tab tile results in highlighter.js errors and an unresponsive toolbox

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Andrei Vaida [:avaida]]

Reproducible on: Nightly 37.0a1 (2015-01-04) *only with* e10s enabled.

Affected platforms: Windows 8.1 64-bit, Ubuntu 12.04 LTS 32-bit, Mac OS X 10.9.5.

Steps to reproduce:
1. Launch Firefox with a clean profile.
2. Make sure e10s is enabled by looking at the "Startup" section from about:preferences.
3. Open a new tab.
4. Click the wrench button from the toolbar and click "Toggle tools".
5. Click the "Pick an element from the page" button from the left side of the toolbox and then select a tile.
6. Check the Browser Console.
7. Click the "Pick an element from the page" button again.
8. Try to close the toolbox.

Expected result:
6. There are no errors thrown for this action.
7. A new element from the page can be selected for inspection.
8. The developer tools pane is closed without causing any issues.

Actual result:
6. Two errors are thrown by highlighter.js:
> TypeError: this._walker is null in highlighter.js:247:4
> TypeError: actor is null in highlighter.js:264:4
7. The same errors are thrown again, multiple times and the "Pick an element from the page" button is no longer working.
8. Clicking the close button from the developer tools is no longer working.

Notes:
- This issue is not reproducible with e10s disabled.

Comment 1

[Patrick Brosset <:pbro>]

I'm not seeing exactly the same errors in the console, but I can reproduce the issue and I'm pretty sure it's the same thing.

Here's the first error I see in the console when this happens:

TypeError: this.docShell is null webbrowser.js:614:4
Error: Connection closed
Stack trace:
Front<.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1113:23
exports.WalkerFront<.destroy@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/inspector.js:2701:5
frontProto/</proto[name]/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1331:11
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:40
then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:11
then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:40:11
then@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:20:43
resolve@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/deprecated-sync-thenables.js:72:11
Front<.onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:1208:7
DebuggerClient.prototype.onPacket@resource://gre/modules/devtools/dbg-client.jsm:897:7
LocalDebuggerTransport.prototype.send/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/transport/transport.js:545:11
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:14
makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/DevToolsUtils.js:83:14
 protocol.js


And here's what seems to be happening:

- The newtab page is hosted in a non-e10s window (for reasons unknown to me), even if the browser is set to use e10s.
- When trying to select a tile using the element picker, instead of selecting the element in the inspector, the tile link is navigated to and the page opens in the same browser tab.
- Since the browser is set to e10s, the window in the browser tab suddenly changes into an e10s window, and this breaks the toolbox in several ways.

In fact, if you do this instead:
1 - Open the new tab page
2 - Open the toolbox
3 - Click on one of the tiles
==> The toolbox is automatically closed, because this toolbox was connected to a non-e10s window, and to use it with the new e10s window that just got loaded, a new connection is required.

So there are 2 things to fix:

- Clicking on a tile when the element picker is turned on should select the element, not load the tile URL.
- Destroying the toolbox while in element picker mode should not break like it does now when closing the toolbox after leaving a non-e10s window. It should correctly shut down.

Comment 2

[Patrick Brosset <:pbro>]

The highlighter (used to pick elements) adds a few event listeners on the parent <xul:browser> of the current page to avoid, e.g. clicks to go through when picking elements.

Here's where the listeners are added:
http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/highlighter.js#267
target here is the parent <xul:browser>

All defined handlers eventually call a function that's supposed to block events from propagating to the page:
http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/actors/highlighter.js#207

It seems that, on the newtab page, the site's href attribute doesn't get blocked by this.
Here's where each site's tile get linked:
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/sites.js#130

Comment 3

[Eddy Bruel [:ejpbruel]]

Mike, any news on this bug?

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

(In reply to Eddy Bruel [:ejpbruel] from comment #3)
> Mike, any news on this bug?

I can't reproduce this anymore... @Andrei: does it work for you?

Comment 5

[Patrick Brosset <:pbro>]

I think part of the problem might have been solved by Alex in bug 1068400. Now, when going from a non-e10s page to an e10s page, the toolbox still closes, but at least, the toolbox closes fine and can be re-opened later.
However, I still see a problem here: when picking an element in a tile, the corresponding URL should *not* open, we should just pick the element for inspection.

Comment 6

[Andrei Vaida [:avaida]]

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #4)
> I can't reproduce this anymore... @Andrei: does it work for you?
I'm no longer seeing this issue on Nightly 39.0a1 (2015-03-28), at least not exactly as described in Comment 0 - instead, there's that lingering problem stated by Patrick in Comment 5. As for the error message, I'm still seeing something similar:
  
  TypeError: this._walker is null highlighter.js:344:4

(tested with Windows 8.1 x64)

Comment 7

[Joe Walker [:jwalker]]

See bug 1193296 for the inspecting a tile issue.