Closed Bug 1478358 Opened Last year Closed 11 months ago

"WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized to a not reachable dimension

Categories

(Testing :: Marionette, defect, P1)

defect

Tracking

(firefox65 fixed, firefox66 fixed)

RESOLVED FIXED
mozilla66
Tracking Status
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: ato, Assigned: whimboo)

References

(Blocks 1 open bug, )

Details

(Keywords: regression)

Attachments

(3 files)

See description and discussion in
https://github.com/mozilla/geckodriver/issues/1326.
Blocks: webdriver
Priority: -- → P3
The patch on bug 1492499 might help here.
I asked the reporter on the github issue for feedback. If it turns out that it works for him now, we can close this bug.
Severity: normal → critical
Keywords: hang
So I actually can reproduce the problem. Here is what happens based on the following script:

> def test_check(driver):
>    driver.set_window_size(1680, 1050)
>    driver.set_window_size(1680, 1050)

The screen size for my external display the tests run on under MacOS is 1680x1050. Now I try to set the window size to that dimension (or higher). The window resizes until it reaches the maximal allowed size, which are the screen dimensions subtracted the height of the menu bar, and the dock. Given that the dock is on the left side for me it results in the following dimensions:

> TRACE	0 -> [0,2,"WebDriver:SetWindowRect",{"height":1050,"width":1680}]
> TRACE	0 <- [1,2,null,{"x":48,"y":22,"width":1632,"height":1028,"state":"maximized"}]

Now with the next call to `set_window_size()` we notice that the requested dimension is different from the current one, and start resizing the window again. But given that it has already reached the maximum size, no more `resize` event will be fired.

What's good is that with bug 1492499 fixed there is no hang anymore. As such we can lower the severity of this bug. But we get a timeout error, which by the spec is not allowed. 

There are two things to fix:

1) Catch and ignore any timeout error for window manipulation commands, and always return success.

2) Only resize a window to a larger dimension if it hasn't reached its maximized state. As it looks like we perfectly detect that already.
Severity: critical → normal
Keywords: hang
OS: Mac OS X → All
Hardware: x86_64 → All
Summary: Resizing window to same dimensions twice hangs Marionette → Resizing window to a not reachable dimension twice throws TimeoutError
Summary: Resizing window to a not reachable dimension twice throws TimeoutError → "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized twice to a not reachable dimension

We have to fix this for the Firefox 65 release, otherwise we will get a lot of complains from users because of those timeout errors.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Priority: P3 → P1
Summary: "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized twice to a not reachable dimension → "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized to a not reachable or the same dimension

Throwing the timeout will actually be fixed by bug 1515867. But running into it which causes a delay similar to minimize window, I will pick up afterward.

Assignee: hskupin → nobody
Status: ASSIGNED → NEW
Depends on: 1515867
Summary: "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized to a not reachable or the same dimension → Delay in "WebDriver:SetWindowRect" if window is resized to a not reachable or the same dimension
Blocks: 1515867
No longer depends on: 1515867

Sorry for the back and forth but we decided to fix this for real and not to get away with the timeout error only.

Assignee: nobody → hskupin
Status: NEW → ASSIGNED
Summary: Delay in "WebDriver:SetWindowRect" if window is resized to a not reachable or the same dimension → "WebDriver:SetWindowRect" throws TimeoutError if window is resized to the same dimension
Summary: "WebDriver:SetWindowRect" throws TimeoutError if window is resized to the same dimension → "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized to a not reachable or the same dimension

I reproduced this now by trying to set a dimension of 100x100 twice for the browser window. Whereby the absolute minimal size can be width=335px, height=121px on my system. As such we are trying to resize but never get a resize event. As such the DebounceCallback never gets run, and early exits.

Maybe we need a maximum expected time until the first event should actually happen. And if that is not the case bail out.

Summary: "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized to a not reachable or the same dimension → "WebDriver:SetWindowRect" throws invalid TimeoutError if window is resized to a not reachable dimension

Ok, so if you call win.resizeTo() the resize operation will be done in a single step. As such only a single resize event is being fired. That means we clearly don't need the debounceCallback method here.

Further and similar to win.moveTo() the function returns when the operation has been done. As such we should be able to even get rid of using the TimedPromise. We should only use the IdlePromise to allow the resize event to be fired.

Here how it looks like when resizing to a new dimension:

1547219997180 Marionette DEBUG 3 -> [0,15,"WebDriver:SetWindowRect",{"y":null,"x":null,"width":335,"height":100}]
** before resizeTo
** before IdlePromise
** resize event
** after IdlePromise
1547219997317 Marionette DEBUG 3 <- [1,15,null,{"x":48,"y":23,"width":335,"height":121}]

With the upcoming patch we will also have a huge performance improvement. I run the Mn unit test test_window_rect.py 10 times, and here is the output:

before: 24.85s user 6.69s system 43% cpu 1:12.92 total
after: 24.83s user 6.70s system 69% cpu 45.294 total

As visible we are way better utilizing the CPU, and as such it is ~38% faster.

To have the same pre-conditions for all wdspec tests, the session fixture
has to set a default window position.

Depends on D16338
Blocks: 1514616
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/602e8cefc900
[marionette] Don't wait for resize events in "Set Window Rect". r=ato
https://hg.mozilla.org/integration/autoland/rev/d4a75d417b03
[wdspec] Enforce a default window position for each session. r=ato
https://hg.mozilla.org/integration/autoland/rev/3ef29704bc3d
[wdspec] Enhance tests for the "Set Window Rect" command. r=ato

This is a test-only regression for Firefox 65. Please uplift to mozilla-beta.

Whiteboard: [checkin-needed-beta]
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14864 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks:
Github PR https://github.com/web-platform-tests/wpt/pull/14864
* Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/NF3aBn3AQP23lDFSCoDbLQ)
Upstream PR was closed without merging
Blocks: 1506095
Blocks: 1492499
No longer depends on: 1492499
Keywords: regression
Blocks: 1516975
Blocks: 1506898
Blocks: 1489955
Blocks: 1507372
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.