Test failure 'controller.waitForPageLoad(URI=about:newtab, readyState=complete)' in /testAwesomeBar/testVisibleItemsMax.js

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Andrei Eftimie, Assigned: Andrei Eftimie)

Tracking

({regression, reproducible})

unspecified
regression, reproducible
Dependency tree / graph

Firefox Tracking Flags

(firefox34 fixed)

Details

(Whiteboard: [mozmill-test-failure], URL)

Attachments

(4 attachments)

(Assignee)

Description

3 years ago
Module:    testVisibleItemsMax
Test:      /testAwesomeBar/testVisibleItemsMax.js    
Failure:   controller.waitForPageLoad(URI=about:newtab, readyState=complete)
Branches:  default
Platforms: All
Report:    http://mozmill-daily.blargon7.com/#/functional/report/4c400675e8fe3ac64bc7502e7636155a

Fails across platforms.
I'll have a skip patch shortly.

I can reproduce it locally. Does not fail if the test is run individually.
Need to run a testrun.

MC pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a91ec42d6a9c&tochange=613e79262240
(Assignee)

Comment 1

3 years ago
Created attachment 8462537 [details] [diff] [review]
skip.patch
Assignee: nobody → andrei.eftimie
Status: NEW → ASSIGNED
Attachment #8462537 - Flags: review?(andreea.matei)
(Assignee)

Updated

3 years ago
status-firefox34: --- → affected
Comment on attachment 8462537 [details] [diff] [review]
skip.patch

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

http://hg.mozilla.org/qa/mozmill-tests/rev/0cf430138cc0 (default)
Attachment #8462537 - Flags: review?(andreea.matei) → review+
status-firefox34: affected → disabled
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][mozmill-test-skipped]
(Assignee)

Comment 3

3 years ago
Pushlog: https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=362b6f577380&tochange=8bb77d17d0ee
Regressor: bug 1040335

This only enabled the unified autocomplete toolbar.
> browser.urlbar.unifiedcomplete

I'll dissect the failure now, maybe prepare a testcase.
Blocks: 1040335
Keywords: regressionwindow-wanted
(Assignee)

Comment 4

3 years ago
Created attachment 8463952 [details]
testcase.js

Testcase attached. Seems there's a race condition.
Once we switch to a tab, if we type to fast and press enter the urlbar does not acknowledge our input at all. This is weird.

There's a commented sleep call in line 37.
With that 100ms sleep everything works fine.
This is weird because the sleep is between typing a new url and hitting enter.

I'll need to debug further to understand why this is failing.
Andrei, do you have any news here? Maybe Marco can help us? I don't see that he has been cc'ed to this bug.
(Assignee)

Comment 6

3 years ago
Created attachment 8467880 [details]
async_title_loading.mov

I found out _why_ this is failing.

Steps (from the testcase):
1. We open a tab, load a page
2. Open a new tab, type in "grants" (this is included in the url from step 1)
3. Click on the "Switch to tab" item from the Autocomplete List
4. After the switch we try typing in something
5. The LocationBar value is getting updated with the original url, overwriting anything we wrote.

Attached is a screencast showcasing the behaviour (this is the second screencast I've done in years, so I'm not sure if a quicktime .mov file is any good for other platforms, used the system default to record on OSX, if anyone needs it converted please let me know and I'll do my best).

The issue is visible between the seconds 11-13.

==

There's no way to reproduce this manually as 100ms seems a strong threshold that makes the testcase pass every time.

Marco, in lieu of the new Unified Autocomplete that's been preffed on in bug 1040335, is this behaviour expected?

If so, are there other particular cases where we might expect the LocationBar to async load the URL? And are there any observers we might want to listen to?
Flags: needinfo?(mak77)
This is really strange, we are switching to an existing tab, not loading it... I wonder if

assert.waitFor(() => controller.tabs.activeTab.location.href === "http://mozqa.com/data/firefox/layout/mozilla_grants.html",

is continuing too early, so if unified is filing in the url in the original tab before the switch to tab happens, then you type before switch to tab has finished doing its job. maybe before we were not setting the value to the urlbar so you were properly expecting the switch operation to complete.
This is just a theory though. what makes me think may be the case, is the fact the switch operation is handled by the browser not by autocomplete.

The other fact that might be related is we use promises/Task and those run on the next tick.

does the test pass if you wait just one tick instead of 100ms?
Flags: needinfo?(mak77)
(Assignee)

Comment 8

3 years ago
(In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from comment #7)
> This is really strange, we are switching to an existing tab, not loading
> it... I wonder if
> 
> assert.waitFor(() => controller.tabs.activeTab.location.href ===
> "http://mozqa.com/data/firefox/layout/mozilla_grants.html",
> 
> is continuing too early, so if unified is filing in the url in the original
> tab before the switch to tab happens, then you type before switch to tab has
> finished doing its job. maybe before we were not setting the value to the
> urlbar so you were properly expecting the switch operation to complete.
> This is just a theory though. what makes me think may be the case, is the
> fact the switch operation is handled by the browser not by autocomplete.

I've been issuing some dumps, here's the interesting cross-section:
(for each row we have: active tab index, controller.tabs.activeTab.location.href, urlbar.value)

> == 2) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html
Here we open a new tab, type in "grants"
> == 3) active tab: 3 : about:newtab -
> == 4) active tab: 3 : about:newtab -
> == 5) active tab: 3
We now click on "switch to tab":
> == 6) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html
> == 7) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html
> == 8) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html
> == 9) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html
Exactly here we type the new url:
> == 10) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - http://mozqa.com/data/firefox/layout/mozilla.html
We hit Enter:
> == 11) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html
> == 12) active tab: 2 : http://mozqa.com/data/firefox/layout/mozilla_grants.html - mozqa.com/data/firefox/layout/mozilla_grants.html

So the activeTab is switched right away when we "Switch to tab", it still has the original URL loaded, but after some time the url gets updated and whatever value we type in between these 2 points gets overwritten.

Updated

3 years ago
Blocks: 995091
(Assignee)

Comment 9

3 years ago
Seems this issue was fixed with the build from 15th August.
Pushlog for fix on mc: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5299864050ee&tochange=c9f8cc9ce89c
(Assignee)

Comment 10

3 years ago
Fix came in from fx-team: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=31db71673c6e&tochange=9eb591a1a203

Where the fix for bug 1051830 came in, which is possible also fixed this issue.
(Assignee)

Comment 11

3 years ago
Confirmed: landing of bug 1051830 did also made this test pass.
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=938003717a25&tochange=2ccc5c9a58fa
Blocks: 1051830
(Assignee)

Comment 12

3 years ago
Created attachment 8477366 [details] [diff] [review]
unskip.patch

Seems the disable line got duplicate somewhere, so a straight backout doesn't work.
Attachment #8477366 - Flags: review?(andreea.matei)
(Assignee)

Comment 13

3 years ago
Both OSX and Windows are green with this patch.
Comment on attachment 8477366 [details] [diff] [review]
unskip.patch

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

Updated the commit message and pushed:
http://hg.mozilla.org/qa/mozmill-tests/rev/03dd84d656d3 (default)
Attachment #8477366 - Flags: review?(andreea.matei) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox34: disabled → fixed
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] → [mozmill-test-failure]
You need to log in before you can comment on or make changes to this bug.