Closed Bug 1093707 Opened 10 years ago Closed 10 years ago

Using navigate() in chrome scope locks up the browser

Categories

(Testing :: Marionette Client and Harness, defect)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: whimboo, Assigned: chmanchester)

References

()

Details

(Keywords: pi-marionette-client, pi-marionette-server)

Attachments

(1 file, 1 obsolete file)

When you use navigate() in chrome scope, the referenced page will replace e.g. the whole browser and the test locks up. You have to manually kill the browser.

So here are the following questions:

1. Should we allow navigate() in chrome scope at all? At least for desktop I don't see that this is helpful, but can damage more if it happens by accident

2. Why does Marionette not survive and force kill the browser after a while? Not sure if there is a max timeout, but waiting for a while it is still hanging for me.

Whatever we wanna do with the above, we should have at least a guard to make sure those things cannot happen.
I think it makes sense to do both of those things you suggest.  We may want to start with the easy bit which is to disallow navigate in chrome context maybe?
On IRC Andrew raised the question if it would be necessary for B2G to load URLs in chrome scope. If that is the case we might have to differentiate for those platforms.
It seems like we can detect when we're in desktop in chrome scope and throw an exception based on that.
Yes, look for this.appName === "desktop".
(In reply to Andreas Tolfsen (:ato) from comment #5)
> Yes, look for this.appName === "desktop".

appName == "Firefox" maybe.=, but yeah.
(In reply to Chris Manchester [:chmanchester] from comment #6)
> (In reply to Andreas Tolfsen (:ato) from comment #5)
> > Yes, look for this.appName === "desktop".
> 
> appName == "Firefox" maybe.=, but yeah.

Oops, yes.
Looks like bug 1094381 might want to help us here for the cases of brokeness, or?
Depends on: 1094381
(In reply to Henrik Skupin (:whimboo) from comment #8)
> Looks like bug 1094381 might want to help us here for the cases of
> brokeness, or?

I don't think any error is thrown for us to catch in these cases, but I think we should send one to the client directly.
/r/715 - Bug 1093707 - Return an error from marionette when attempting to navigate in chrome scope.

Pull down this commit:

hg pull review -r 95dcf7c98e0937a1d57f726a8ff0df0df59422b4
Assignee: nobody → cmanchester
https://reviewboard.mozilla.org/r/713/#review317

::: testing/marionette/marionette-server.js
(Diff revision 1)
> +      sendError("Cannot navigate in chrome context", 13, null, command_id);

I'm not happy with "unknown error" for this one, but there seem to be no better alternatives in the Selenium protocol.

We should be using the "unsupported operation" status code that is defined in the WD specification, but that doesn't have a corresponding status code number in Selenium which we currently use for defining errors.

Can you please add a TODO and file a bug that we need to revisit this and make it block bug 945729?
/r/715 - Bug 1093707 - Return an error from marionette when attempting to navigate in chrome scope.

Pull down this commit:

hg pull review -r e4211b6a4e92d4b460abfddf7d517d684e602dde
Attachment #8523910 - Flags: review?(ato) → review+
https://hg.mozilla.org/mozilla-central/rev/f597a679642a
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
No longer depends on: 1094381
Attachment #8523910 - Attachment is obsolete: true
Attachment #8618550 - Flags: review+
Product: Testing → Remote Protocol

Moving bugs for Marionette client due to component changes.

Component: Marionette → Marionette Client and Harness
Product: Remote Protocol → Testing
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.