Fix script_timeout regressions for Execute (Async) Script since the landing of bug 1510929

RESOLVED FIXED in Firefox 66

Status

defect
P1
normal
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: whimboo, Assigned: whimboo)

Tracking

({regression})

Trunk
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox65 unaffected, firefox66 fixed, firefox67 fixed)

Details

Attachments

(2 attachments)

Since bug 1510929 landed both the execute script and execute async script are broken in the following ways:

  1. The custom timeout as set isn't reset if the script runs into a timeout

  2. Fractions of a second are lost by script_timeout / 1000 because it's a decimal division. As such a timeout of 100ms will result in 0ms.

This fixes the following regressions as introduced by
bug 1510929 for the Marionette client.

  1. The custom timeout as set isn't reset if the
    script times out.

  2. Fractions of a second for the script timeout are
    lost because the division operates on decimal valus.
    As such a timeout of 100ms will result in 0ms.

Fractions of a second are lost because the division
in getting the timeout value operates on decimal valus.
As such a timeout of 100ms will result in 0ms.

Depends on D18214

Comment 4

4 months ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/934099e1c866
[marionette] Correctly handle script_timeout for WebDriver:{ExecuteScript,ExecuteAsyncScript}. r=jgraham
https://hg.mozilla.org/integration/autoland/rev/71f896682a2f
[marionette] Correct calculation of timeout from milliseconds to seconds. r=jgraham

Comment 5

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Please uplift the test-only patch to mozilla-beta to fix this regression.

Whiteboard: [checkin-needed-beta]

:whimboo can you, please, point me to the test-only patch?

Flags: needinfo?(hskupin)
Flags: needinfo?(hskupin)
Whiteboard: [checkin-needed-beta]
You need to log in before you can comment on or make changes to this bug.