Improve error handling in JavaUrlRequest

Improve error handling in JavaUrlRequest

BUG=643771
Committed: https://crrev.com/5912b2fa466b153ecafe669ea792d790751d91f0
Cr-Commit-Position: refs/heads/master@{#416367}

[Charles, 2016-09-02 20:59:38]

Description was changed from

==========
Improve error handling in JavaUrlRequest

BUG=643771
==========

to

==========
Improve error handling in JavaUrlRequest

BUG=643771
==========

[Charles, 2016-09-02 20:59:38]

clm@google.com changed reviewers:
+ xunjieli@chromium.org

[Charles, 2016-09-02 20:59:49]

PTAL

[xunjieli, 2016-09-02 21:05:01]

https://codereview.chromium.org/2302253003/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/JavaUrlRequest.java
(right):

https://codereview.chromium.org/2302253003/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/JavaUrlRequest.java:444:
while (true) {
I don't understand the need for a while-true loop. Could you elaborate?

[Charles, 2016-09-02 21:08:36]

https://codereview.chromium.org/2302253003/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/JavaUrlRequest.java
(right):

https://codereview.chromium.org/2302253003/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/JavaUrlRequest.java:444:
while (true) {
On 2016/09/02 21:05:01, xunjieli wrote:
> I don't understand the need for a while-true loop. Could you elaborate?

We're doing a compare-and-set continuously until we either observe a terminal
state, or successfully set a terminal state. The loop only continues if we
observe a non-terminal state, like READING, but when we try to compare-and-set
READING to ERROR, the current state changed to something else. Then we repeat
the process. In practice, this loop will almost never run more than once, and it
can't run forever because the only way to have the CAS fail is for the system to
make progress towards a completed state.

[xunjieli, 2016-09-02 21:41:14]

The diffs LGTM, though I haven't had the time to look through the entire
JavaCronetEngine code to be certain that this will fix the flake.

https://codereview.chromium.org/2302253003/diff/20001/components/cronet/andro...
File components/cronet/android/api/src/org/chromium/net/JavaUrlRequest.java
(right):

https://codereview.chromium.org/2302253003/diff/20001/components/cronet/andro...
components/cronet/android/api/src/org/chromium/net/JavaUrlRequest.java:444:
while (true) {
On 2016/09/02 21:08:36, Charles wrote:
> On 2016/09/02 21:05:01, xunjieli wrote:
> > I don't understand the need for a while-true loop. Could you elaborate?
> 
> We're doing a compare-and-set continuously until we either observe a terminal
> state, or successfully set a terminal state. The loop only continues if we
> observe a non-terminal state, like READING, but when we try to compare-and-set
> READING to ERROR, the current state changed to something else. Then we repeat
> the process. In practice, this loop will almost never run more than once, and
it
> can't run forever because the only way to have the CAS fail is for the system
to
> make progress towards a completed state.

Acknowledged.

[Charles, 2016-09-02 21:42:11]

The CQ bit was checked by clm@google.com

[commit-bot: I haz the power, 2016-09-02 21:42:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-02 22:26:47]

Description was changed from

==========
Improve error handling in JavaUrlRequest

BUG=643771
==========

to

==========
Improve error handling in JavaUrlRequest

BUG=643771
==========

[commit-bot: I haz the power, 2016-09-02 22:26:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-02 22:28:42]

Description was changed from

==========
Improve error handling in JavaUrlRequest

BUG=643771
==========

to

==========
Improve error handling in JavaUrlRequest

BUG=643771

Committed: https://crrev.com/5912b2fa466b153ecafe669ea792d790751d91f0
Cr-Commit-Position: refs/heads/master@{#416367}
==========

[commit-bot: I haz the power, 2016-09-02 22:28:43]

Patchset 2 (id:??) landed as
https://crrev.com/5912b2fa466b153ecafe669ea792d790751d91f0
Cr-Commit-Position: refs/heads/master@{#416367}