[e10s] cookie list fails

VERIFIED FIXED in Firefox 45

Status

()

Firefox
Developer Tools: Graphic Commandline and Toolbar
P1
normal
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: Geoffroy Planquart, Assigned: pbro)

Tracking

(Blocks: 2 bugs)

39 Branch
Firefox 45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+, firefox45 verified)

Details

(Whiteboard: [polish-backlog][difficulty=medium])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150310004228

Steps to reproduce:

./mach build
./mach run
press shift + F2 to open gcli
type "cookie list" then enter


Actual results:

On UI: TypeError: this.window is null

In the program output:

console.error: 
  Message: TypeError: this.window is null
  Stack:
    CommandUtils.createEnvironment/<.document@resource:///modules/devtools/DeveloperToolbar.jsm:224:9
exports.items<.exec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/commands/cookie.js:48:11
Requisition.prototype.exec/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/cli.js:2098:16
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:239:18
Task_spawn@resource://gre/modules/Task.jsm:164:12
exports.exec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/host.js:72:10
Requisition.prototype.exec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/cli.js:2097:14
Inputter.prototype._handleReturn@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/mozui/inputter.js:563:12
Inputter.prototype.handleKeyUp@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/mozui/inputter.js:462:1
Inputter.prototype.onKeyUp@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/mozui/inputter.js:434:3


Expected results:

It should have listed cookies or told me that there aren't any cookies.
(Reporter)

Updated

3 years ago
Blocks: 1042859
Component: Untriaged → Developer Tools: Graphic Commandline and Toolbar
Seems e10s-related.
Blocks: 875871
Status: UNCONFIRMED → NEW
tracking-e10s: --- → ?
Ever confirmed: true
I can repro on Windows, too -> platform/os = all/all.
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Comment 3

3 years ago
GCLI isn't compatible with e10s just yet, and I'm almost certain that's what's causing this issue.
We're getting there. jwalker is working on this in bug 1128988.
(Reporter)

Comment 4

3 years ago
Thanks for the feedback.

Quick & dirty fix which surprisingly works:

diff --git a/browser/devtools/shared/DeveloperToolbar.jsm b/browser/devtools/shared/DeveloperToolbar.jsm
index 9c2375e..ded0d95 100644
--- a/browser/devtools/shared/DeveloperToolbar.jsm
+++ b/browser/devtools/shared/DeveloperToolbar.jsm
@@ -217,7 +217,7 @@ let CommandUtils = {
       },
 
       get window() {
-        return this.chromeWindow.gBrowser.selectedBrowser.contentWindow;
+        return this.chromeWindow.gBrowser.selectedBrowser._contentWindow;
       },
 
       get document() {
No longer blocks: 1042859
tracking-e10s: ? → +

Comment 5

3 years ago
Build:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0 ID:20150516030207

Current results:
"cookieMgr is not defined"
I get the following error in Nightly 41:

Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]

resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/commands/cookie.js:13:10
XPCU_defineLazyGetter/<.get@resource://gre/modules/XPCOMUtils.jsm:196:21
exports.items<.exec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/commands/cookie.js:70:11
Requisition.prototype.exec/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/cli.js:2073:16
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:239:18Task_spawn@resource://gre/modules/Task.jsm:164:12
exports.exec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/util/host.js:72:10
Requisition.prototype.exec@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/cli.js:2072:14
Requisition.prototype.updateExec/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/gcli/cli.js:2112:12
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23

which points to the same trouble with the cookie service that the storage inspector is having.  We'll probably solve the issue there, so let's depend on it.
Depends on: 1049888
Summary: window is null in createEnvironment → [e10s] cookie list fails

Comment 7

3 years ago
I have this in version 40.0a2 - first I get:

Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)

and then if I try 'cookie list' again I get:

cookieMgr is not defined
Blocks: 1150104
Duplicate of this bug: 1202074
Duplicate of this bug: 1204334
Whiteboard: [polish-backlog]
Priority: -- → P1
Whiteboard: [polish-backlog] → [polish-backlog][difficulty=medium]
(Assignee)

Comment 10

2 years ago
My understanding of the problem is that nsICookieManager2 isn't available in child processes.
However the 'cookie list' command is set to run on the server (same with the 'cookie remove' and 'cookie set' commands).
Running on the server makes sense because we need the command to run on the debuggee, which could be a remote device. But that being said, once the command starts executing on the server, it should then reach out to its parent process to get the list of cookies from there.

You can confirm this by changing 'runAt: "server"' to 'runAt: "client"' in the cookie.js gcli commands file and executing 'cookie list' on firefox desktop with e10s.

This is very similar to what the storageActor does.
The command would need to setup a script in the parent process, using something like:

    this.conn.setupInParent({
      module: "path/to/the/cookie-command-script-in-parent",
      setupParent: "setupMethodName"
    });

(by the way, path/to/the/cookie-command-script-in-parent could be the command file itself to avoid having to create another one).

And then, setupMethodName should start listening to child process messages like list, remove, set and respond accordingly.
(Assignee)

Comment 11

2 years ago
(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #10)
>     this.conn.setupInParent({
>       module: "path/to/the/cookie-command-script-in-parent",
>       setupParent: "setupMethodName"
>     });
The problem here is that there's potentially no active DebuggerServer connections, the toolbox might not have been opened before. So I don't think setupInParent can be used.

Joe, can gcli help server commands run code in the server's parent process? Do you think having a 'runAt: "server-parent"' mode would be possible?
Flags: needinfo?(jwalker)
runAt: "server-parent" is a really good idea.

A temporary workaround might be to say runAt:client and then set the command to hidden:isFxDesktop or something nasty like that. That way toolboxes that are talking to a non desktop process won't have the command. Ug, though.
Flags: needinfo?(jwalker)
(Assignee)

Comment 13

2 years ago
(In reply to Joe Walker [:jwalker] (needinfo me or ping on irc) from comment #12)
> runAt: "server-parent" is a really good idea.
Filed bug 1221488 for it.
(Assignee)

Comment 14

2 years ago
Created attachment 8683651 [details] [diff] [review]
Bug_1142292_-_Run_cookie_gcli_commands_on_the_clie.diff

Would you say this is an acceptable work-around until bug 1221488 lands?
The developer toolbar is only accessible on desktop firefox anyway, and the cookie commands aren't called from anywhere else today. So we can safely assume that they always run on a desktop firefox when debugging a local tab.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Attachment #8683651 - Flags: review?(jwalker)
Comment on attachment 8683651 [details] [diff] [review]
Bug_1142292_-_Run_cookie_gcli_commands_on_the_clie.diff

Review of attachment 8683651 [details] [diff] [review]:
-----------------------------------------------------------------

Is there an easy way to say add

    get hidden() {
      return !isRemoteFirefoxDesktop;
    }

To the command definition?

i.e. a variant of https://dxr.mozilla.org/mozilla-central/source/devtools/shared/gcli/commands/calllog.js#103

    get hidden() {
      return gcli.hiddenByChromePref();
    },

So we could hide the command in places where it wouldn't work. I'm guessing that's not easy.
Attachment #8683651 - Flags: review?(jwalker) → review+
(Assignee)

Comment 16

2 years ago
(In reply to Joe Walker [:jwalker] (needinfo me or ping on irc) from comment #15)
> Comment on attachment 8683651 [details] [diff] [review]
> Bug_1142292_-_Run_cookie_gcli_commands_on_the_clie.diff
> 
> Review of attachment 8683651 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Is there an easy way to say add
> 
>     get hidden() {
>       return !isRemoteFirefoxDesktop;
>     }
> 
> To the command definition?
> 
> i.e. a variant of
> https://dxr.mozilla.org/mozilla-central/source/devtools/shared/gcli/commands/
> calllog.js#103
> 
>     get hidden() {
>       return gcli.hiddenByChromePref();
>     },
> 
> So we could hide the command in places where it wouldn't work. I'm guessing
> that's not easy.

I've been trying to find a way to do this, maybe the command could check if there's an active toolbox with a remote connection or something. But the thing is, the developertoolbar doesn't work with remote connections. When you open it, it always targets the local firefox tab it's attached to. So there's no way for it to execute commands over a remote connection.

So the only risk here is if someone tries to execute the cookie commands from toolbox buttons (which can run commands on remote connections). Someone trying to do this would have to modify the cookie.js file and would find out about the restriction.
But in order to further mitigate this, I guess I could add in all `exec` functions a check for:
context.environment.target.isRemote
and if `true`, then throw a developer-friendly error.
(Assignee)

Comment 17

2 years ago
Created attachment 8684138 [details] [diff] [review]
Bug_1142292_-_Run_cookie_gcli_commands_on_the_clie.diff

I know that you already R+'d this, but this is just to make sure my reasoning in comment 16 is sane.
Attachment #8683651 - Attachment is obsolete: true
Attachment #8684138 - Flags: review?(jwalker)
(Assignee)

Comment 18

2 years ago
Interdiff: https://bugzilla.mozilla.org/attachment.cgi?oldid=8683651&action=interdiff&newid=8684138&headers=1
Comment on attachment 8684138 [details] [diff] [review]
Bug_1142292_-_Run_cookie_gcli_commands_on_the_clie.diff

Review of attachment 8684138 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks.
Attachment #8684138 - Flags: review?(jwalker) → review+
(Assignee)

Comment 20

2 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d4893a11eccc
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 21

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/beb71b4e9def
Keywords: checkin-needed

Comment 22

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/beb71b4e9def
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox45: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45

Comment 23

2 years ago
I have reproduced this bug with Firefox nightly 44.0a1(build id:20150310004228)on 
windows 7(64 bit)

I have verified this bug as fixed with Firefox beta 45.0b6(build id:20160215141016)
User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.

[testday-20160219]
(In reply to nuz.tasneem from comment #23)
> I have reproduced this bug with Firefox nightly 44.0a1(build
> id:20150310004228)on 
> windows 7(64 bit)
> 
> I have verified this bug as fixed with Firefox beta 45.0b6(build
> id:20160215141016)
> User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101
> Firefox/45.
> 
> [testday-20160219]

Thanks!
Status: RESOLVED → VERIFIED
status-firefox45: fixed → verified
You need to log in before you can comment on or make changes to this bug.