predictors: Don't cancel the request when IO is pending.

predictors: Don't cancel the request when IO is pending.

In resource_prefetch_predictor, when reading the response of a prefetch
request, the request is marked as failed (and aborted) when the status
is IO_PENDING. This is not intended.

BUG=629424
Committed: https://crrev.com/dbcaba59119dd7d4efaf71f4a0ab9e3a410b6e7c
Cr-Commit-Position: refs/heads/master@{#408614}

[Benoit L, 2016-07-28 16:27:12]

lizeb@chromium.org changed reviewers:
+ pasko@chromium.org

[pasko, 2016-07-29 09:35:29]

nice find!

https://codereview.chromium.org/2188253002/diff/1/chrome/browser/predictors/r...
File chrome/browser/predictors/resource_prefetcher.cc (right):

https://codereview.chromium.org/2188253002/diff/1/chrome/browser/predictors/r...
chrome/browser/predictors/resource_prefetcher.cc:180: } else if
(!request->status().is_success()) {
I could be missing something .. but will this not turn into a busy loop at
IO_PENDING?

The async read that we started on line 176 will only change the
request->status() before the OnReadCompleted() is called, but we are occupying
the thread here for it to happen.

Seems like we should break out of the loop and continue in OnReadCompleted().

[pasko, 2016-07-29 09:40:42]

lgtm

https://codereview.chromium.org/2188253002/diff/1/chrome/browser/predictors/r...
File chrome/browser/predictors/resource_prefetcher.cc (right):

https://codereview.chromium.org/2188253002/diff/1/chrome/browser/predictors/r...
chrome/browser/predictors/resource_prefetcher.cc:180: } else if
(!request->status().is_success()) {
On 2016/07/29 09:35:29, pasko wrote:
> I could be missing something .. but will this not turn into a busy loop at
> IO_PENDING?
> 
> The async read that we started on line 176 will only change the
> request->status() before the OnReadCompleted() is called, but we are occupying
> the thread here for it to happen.
> 
> Seems like we should break out of the loop and continue in OnReadCompleted().

oh, my reading was wrong, the status is actually false and we will break out of
the loop.

[Benoit L, 2016-07-29 09:44:56]

The CQ bit was checked by lizeb@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-29 09:45:07]

Dry run: 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-07-29 10:24:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 10:24:25]

Dry run: This issue passed the CQ dry run.

[Benoit L, 2016-07-29 10:49:10]

The CQ bit was checked by lizeb@chromium.org

[commit-bot: I haz the power, 2016-07-29 10:49:28]

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-07-29 10:53:11]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-29 10:55:06]

Description was changed from

==========
predictors: Don't cancel the request when IO is pending.

In resource_prefetch_predictor, when reading the response of a prefetch
request, the request is marked as failed (and aborted) when the status
is IO_PENDING. This is not intended.

BUG=629424
==========

to

==========
predictors: Don't cancel the request when IO is pending.

In resource_prefetch_predictor, when reading the response of a prefetch
request, the request is marked as failed (and aborted) when the status
is IO_PENDING. This is not intended.

BUG=629424

Committed: https://crrev.com/dbcaba59119dd7d4efaf71f4a0ab9e3a410b6e7c
Cr-Commit-Position: refs/heads/master@{#408614}
==========

[commit-bot: I haz the power, 2016-07-29 10:55:07]

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