Retry license request on timeouts and errors

Retry license request on timeouts and errors

Also removes the retry limit so that it will retry on failures until the test framework stops the test.

BUG=418952
Committed: https://crrev.com/a65005b70a2e6783af4d27c8c3f139da649b5783
Cr-Commit-Position: refs/heads/master@{#297741}

[jrummell, 2014-10-01 17:18:25]

jrummell@chromium.org changed reviewers:
+ ddorwin@chromium.org, sandersd@chromium.org

[jrummell, 2014-10-01 17:18:25]

PTAL.

[sandersd (OOO until July 31), 2014-10-01 17:46:12]

https://codereview.chromium.org/614423002/diff/1/media/test/data/eme_player_j...
File media/test/data/eme_player_js/utils.js (right):

https://codereview.chromium.org/614423002/diff/1/media/test/data/eme_player_j...
media/test/data/eme_player_js/utils.js:233: REQUEST_RETRY_DELAY_MS + 'ms');
statusText is typically uninformative (it's probably always going to be the
empty string), so the reason should still be in the message.

https://codereview.chromium.org/614423002/diff/1/media/test/data/eme_player_j...
media/test/data/eme_player_js/utils.js:250: Utils.timeLog('Attempt (' +
requestAttemptCount + ') timeout');
It would be good to format the three kinds of failure as similarly as possible.

[jrummell, 2014-10-01 20:49:07]

Updated.

https://codereview.chromium.org/614423002/diff/1/media/test/data/eme_player_j...
File media/test/data/eme_player_js/utils.js (right):

https://codereview.chromium.org/614423002/diff/1/media/test/data/eme_player_j...
media/test/data/eme_player_js/utils.js:233: REQUEST_RETRY_DELAY_MS + 'ms');
On 2014/10/01 17:46:12, sandersd wrote:
> statusText is typically uninformative (it's probably always going to be the
> empty string), so the reason should still be in the message.

Done.

https://codereview.chromium.org/614423002/diff/1/media/test/data/eme_player_j...
media/test/data/eme_player_js/utils.js:250: Utils.timeLog('Attempt (' +
requestAttemptCount + ') timeout');
On 2014/10/01 17:46:11, sandersd wrote:
> It would be good to format the three kinds of failure as similarly as
possible.

Done.

[ddorwin, 2014-10-01 21:00:29]

https://codereview.chromium.org/614423002/diff/20001/media/test/data/eme_play...
File media/test/data/eme_player_js/utils.js (right):

https://codereview.chromium.org/614423002/diff/20001/media/test/data/eme_play...
media/test/data/eme_player_js/utils.js:217: var MAXIMUM_REQUEST_ATTEMPTS = 3;
If we're seeing this sometimes requiring 3 locally, should we up this to 4 so
that we never (hopefully) see this on VMs?

https://codereview.chromium.org/614423002/diff/20001/media/test/data/eme_play...
media/test/data/eme_player_js/utils.js:235: setTimeout(sendRequestAttempt,
REQUEST_RETRY_DELAY_MS);
Why do we wait? Is there an advantage to fewer retries spaced further apart (vs.
closer together but more attempts)?

[jrummell, 2014-10-01 21:28:34]

Updated to remove the retry limit.

https://codereview.chromium.org/614423002/diff/20001/media/test/data/eme_play...
File media/test/data/eme_player_js/utils.js (right):

https://codereview.chromium.org/614423002/diff/20001/media/test/data/eme_play...
media/test/data/eme_player_js/utils.js:217: var MAXIMUM_REQUEST_ATTEMPTS = 3;
On 2014/10/01 21:00:29, ddorwin wrote:
> If we're seeing this sometimes requiring 3 locally, should we up this to 4 so
> that we never (hopefully) see this on VMs?

Removed this so we try until the framework decides it's taking too long.

https://codereview.chromium.org/614423002/diff/20001/media/test/data/eme_play...
media/test/data/eme_player_js/utils.js:235: setTimeout(sendRequestAttempt,
REQUEST_RETRY_DELAY_MS);
On 2014/10/01 21:00:28, ddorwin wrote:
> Why do we wait? Is there an advantage to fewer retries spaced further apart
(vs.
> closer together but more attempts)?

In this case the license server isn't up, so we might as well wait before trying
again.

[ddorwin, 2014-10-01 21:31:28]

Thanks. High level LGTM once sandersd is happy.

[sandersd (OOO until July 31), 2014-10-01 21:34:19]

lgtm

[jrummell, 2014-10-01 21:35:33]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2014-10-01 21:36:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614423002/40001

[commit-bot: I haz the power, 2014-10-01 23:56:05]

Committed patchset #3 (id:40001) as 52d23099e9206097ba60e540429222a41eba3bf1

[commit-bot: I haz the power, 2014-10-01 23:56:57]

Patchset 3 (id:??) landed as
https://crrev.com/a65005b70a2e6783af4d27c8c3f139da649b5783
Cr-Commit-Position: refs/heads/master@{#297741}