New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 644444 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

[Offline Pages] Download notification is completed upon a canceled attempt due to network connectivity issue or transition to foreground

Project Member Reported by dougarnett@chromium.org, Sep 6 2016

Issue description

I switched to airplane mode directly after offliner attempt started. The attempt was canceled as expected but the notification thought the operation was complete even though it was subject to retry (and did in fact succeed later). 

I think we should not be calling NotifyCompleted for FOREGROUND_CANCELED or PRERENDERING_CANCELED case in OfflinerDoneCallback. We should wait to call it when the attempts expired and request is removed from the queue.

Or else we need to introduce some intermediate status if we want to reveal the attempt.
 
Good point. I was trying to remove cases where we report final status multiple times, must have missed these.

This only affects cases that have retry enabled, right?
Right, some cases of current operation being "canceled" rather than the attempt completing as "failed". We will retry in these canceled cases currently.
If this can happen in M54 (do we have any scenario in M54 that actually tries more than once?) then it'd be M54 bug.
Labels: M-54
Labels: -Pri-2 Pri-1
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9c01aee8aef23aaa838c71fc2df65254b344131

commit f9c01aee8aef23aaa838c71fc2df65254b344131
Author: dougarnett <dougarnett@chromium.org>
Date: Thu Sep 08 17:38:16 2016

[Offline Pages] Simple, mergable fix to no longer report a request completed when it is FOREGROUND_CANCELED or PRERENDERING_CANCELED and subject to retry.

BUG= 644444 

Review-Url: https://codereview.chromium.org/2326593002
Cr-Commit-Position: refs/heads/master@{#417328}

[modify] https://crrev.com/f9c01aee8aef23aaa838c71fc2df65254b344131/components/offline_pages/background/request_coordinator.cc

Labels: Merge-Request-54
Status: Fixed (was: Assigned)
Fixed in origin

Comment 8 by dimu@chromium.org, Sep 8 2016

Labels: -Merge-Request-54 Merge-Approved-54 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M54 (branch: 2840)
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 8 2016

Labels: -merge-approved-54 merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6a47a78adea9f959e5df0ea4b21279abc78055f

commit e6a47a78adea9f959e5df0ea4b21279abc78055f
Author: Pete Williamson <petewil@chromium.org>
Date: Thu Sep 08 18:00:15 2016

[Offline Pages] Simple, mergable fix to no longer report a request completed when it is FOREGROUND_CANCELED or PRERENDERING_CANCELED and subject to retry.

BUG= 644444 

Review-Url: https://codereview.chromium.org/2326593002
Cr-Commit-Position: refs/heads/master@{#417328}
(cherry picked from commit f9c01aee8aef23aaa838c71fc2df65254b344131)

Review URL: https://codereview.chromium.org/2324883002 .

Cr-Commit-Position: refs/branch-heads/2840@{#236}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/e6a47a78adea9f959e5df0ea4b21279abc78055f/components/offline_pages/background/request_coordinator.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e6a47a78adea9f959e5df0ea4b21279abc78055f

commit e6a47a78adea9f959e5df0ea4b21279abc78055f
Author: Pete Williamson <petewil@chromium.org>
Date: Thu Sep 08 18:00:15 2016

[Offline Pages] Simple, mergable fix to no longer report a request completed when it is FOREGROUND_CANCELED or PRERENDERING_CANCELED and subject to retry.

BUG= 644444 

Review-Url: https://codereview.chromium.org/2326593002
Cr-Commit-Position: refs/heads/master@{#417328}
(cherry picked from commit f9c01aee8aef23aaa838c71fc2df65254b344131)

Review URL: https://codereview.chromium.org/2324883002 .

Cr-Commit-Position: refs/branch-heads/2840@{#236}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/e6a47a78adea9f959e5df0ea4b21279abc78055f/components/offline_pages/background/request_coordinator.cc

Sign in to add a comment