Closed Bug 1329556 Opened 3 years ago Closed 3 years ago

Navigate inside a frame causes timeout if requested page is equal to the top-level browsing context one

Categories

(Testing :: Marionette, defect)

53 Branch
defect
Not set

Tracking

(firefox51 unaffected, firefox52 fixed, firefox53 fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox51 --- unaffected
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: titus.fortner, Assigned: whimboo)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36

Steps to reproduce:

Geckodriver 0.13.0

Navigate to a page with a frame 2x, switch to a frame, then navigate again.

(weird combination, but it happened in my test suite, and this is what I boiled it down to that is consistently failing)


Actual results:

The navigation command never returns.

https://gist.github.com/titusfortner/91db2faf2a14d9028e579c88946450f5

It appears to be a race condition of some kind. If I put a 1 second sleep before the final navigation call it works just fine. (0.5 seconds does not)


Expected results:

W3C command should have returned null
Attached file minimized testcase
I can reproduce this behavior with this minimized testcase directly with Marionette. I think the problem here is the navigation inside the frame and that we do not see its loaded state.
The spec says:

> 7. Navigate the current top-level browsing context to url. 

Can I assume that calling navigate() inside a frame whould switch to the parent frame first before starting the get request?
Flags: needinfo?(ato)
Flags: needinfo?(ato)
Summary: navigate from frame → Navigate after switching to frame
Summary: Navigate after switching to frame → Navigate after switching to frame causes timeout
(In reply to Henrik Skupin (:whimboo) from comment #2)
> The spec says:
> 
> > 7. Navigate the current top-level browsing context to url. 
> 
> Can I assume that calling navigate() inside a frame whould switch to the
> parent frame first before starting the get request?

That’s in any case what we _should_ be doing.  Navigating whilst the current browsing context is an <iframe>, should move the current browsing context to the top-level browsing context and then navigate.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can take this as one of my next items.
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
It looks like that we had this already fixed in bug 977899. But given that the associated unit test hasn't been re-enabled we seem to have regressed it again.
Ok, so the issue here is not that we are staying in the nested browsing context, but when we determine if a page load event is expected. Given that we currently change into the top-level browsing context too late we compare the requested URL with the one from the frame, and as such wait for a load event. But given that the URL is identical to the one as loaded before, no load event will occur. As such we run into the timeout.
Summary: Navigate after switching to frame causes timeout → Navigate inside a frame causes timeout if requested page is equal to the top-level browsing page
Summary: Navigate inside a frame causes timeout if requested page is equal to the top-level browsing page → Navigate inside a frame causes timeout if requested page is equal to the top-level browsing context one
Setting this to fix-optional to get it off the regression bug triage radar.
Attachment #8825399 - Flags: review?(dburns)
Comment on attachment 8825399 [details]
Bug 1329556 - Navigation has to switch to top frame before determining load events.

https://reviewboard.mozilla.org/r/103574/#review105664
Attachment #8825399 - Flags: review?(dburns) → review+
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3eb974e77d50
Navigation has to switch to top frame before determining load events. r=automatedtester
https://hg.mozilla.org/mozilla-central/rev/3eb974e77d50
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Please uplift this test-only patch to aurora. Thanks.
Whiteboard: [checkin-needed-aurora]
You need to log in before you can comment on or make changes to this bug.