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

Issue 704726 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit 22 days ago
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Why do we have an error for Already Exists when saving an offline page?

Project Member Reported by petewil@chromium.org, Mar 23 2017

Issue description

We should either make this a success case or save over the existing page.

When offlining, we could check the offline page model, and if we see that we already have a page, just return success.

Alternately, we could allow saving a page over an existing page, and remove the old page when saving the newer one.
 
Labels: Hotlist-Fixit
Discussion in the group was that we should either leave this alone (as it can help us find bugs), or fix the offline page model to allow updates, and to inform observers (there is currently no updated method on the observers).  If we do that, we should write a design doc first.

Comment 3 by dim...@chromium.org, Mar 29 2017

Labels: OS-Android
Owner: chili@chromium.org
Status: Assigned (was: Untriaged)
This looks like it might indeed be the proper fix for the ErrorAlreadyExists case when saving MHTML.
Cc: petewil@chromium.org
 Issue 674702  has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 25 2017

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

commit 85242f08db71684cbba4bcf94cce92ed91d9726f
Author: petewil <petewil@chromium.org>
Date: Tue Apr 25 17:10:29 2017

Treat already existing saved pages as success.

In some cases, we ran into a failure mode where we saved off a copy of
a page, but then failed.  When we retried, we were getting a result of
ALREADY_EXISTS for the page, but we treated it as an error, which
meant we could never succeed for that page.  We now treat ALREADY_EXISTS
as a success case.  Added a new unit test to check the new code.

BUG= 704726 

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

[modify] https://crrev.com/85242f08db71684cbba4bcf94cce92ed91d9726f/chrome/browser/android/offline_pages/background_loader_offliner.cc
[modify] https://crrev.com/85242f08db71684cbba4bcf94cce92ed91d9726f/chrome/browser/android/offline_pages/background_loader_offliner_unittest.cc
[modify] https://crrev.com/85242f08db71684cbba4bcf94cce92ed91d9726f/chrome/browser/android/offline_pages/prerendering_offliner.cc

Comment 7 by chili@chromium.org, Jun 20 2017

Status: Fixed (was: Assigned)

Sign in to add a comment