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

Issue 717706 link

Starred by 1 user
Status: Fixed
Owner:
█ chili@chromium.org
Last visit 22 days ago
Closed: May 2017
Cc:
fgor...@chromium.org
petewil@chromium.org
carlosk@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

All error codes from the MHTML archiver are being reported as ArchiveCreationFailed

Project Member Reported by chili@chromium.org, May 2 2017 
We have a check in the offline_page_model_impl where if the url we saved (returned from the mhtml archiver) is different from the url we expected to save, then we override whatever error code is returned from the archiver and return archive_creation_failed.

Consequently, the mhtml archiver is returning GURL() (empty url) whenever it detected a failure, along with a different flavored error code.  This causes us to always report ARCHIVE_CREATION_FAILED no matter the real reason even in UMA
Project Member

Comment 1 by bugdroid1@chromium.org, May 3 2017 

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

commit 7ed8f251fa9ae767f20a32abe8d70921b883d062
Author: chili <chili@chromium.org>
Date: Wed May 03 19:46:08 2017

[Offline pages] Fix order of error checking so URL mismatch would not override other error codes

BUG= 717706 

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

[modify] https://crrev.com/7ed8f251fa9ae767f20a32abe8d70921b883d062/components/offline_pages/core/offline_page_model_impl.cc
[modify] https://crrev.com/7ed8f251fa9ae767f20a32abe8d70921b883d062/components/offline_pages/core/offline_page_model_impl.h

Comment 2 by chili@chromium.org, May 3 2017

Labels: Merge-Request-59
Status: Fixed (was: Started)

Comment 3 by abdulsyed@chromium.org, May 3 2017 

Can you please mark which OS this fix is affecting?

Comment 4 by chili@chromium.org, May 3 2017

Labels: OS-Android
Project Member

Comment 5 by sheriffbot@chromium.org, May 3 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 6 by bugdroid1@chromium.org, May 3 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21f8e7b001337051a0bfa84c30f53ce557c99d1b

commit 21f8e7b001337051a0bfa84c30f53ce557c99d1b
Author: Justin DeWitt <dewittj@chromium.org>
Date: Wed May 03 20:34:36 2017

[Offline pages] Fix order of error checking so URL mismatch would not override other error codes

BUG= 717706 

Review-Url: https://codereview.chromium.org/2860543002
Cr-Commit-Position: refs/heads/master@{#469082}
(cherry picked from commit 7ed8f251fa9ae767f20a32abe8d70921b883d062)

Review-Url: https://codereview.chromium.org/2856053003 .
Cr-Commit-Position: refs/branch-heads/3071@{#381}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/21f8e7b001337051a0bfa84c30f53ce557c99d1b/components/offline_pages/core/offline_page_model_impl.cc
[modify] https://crrev.com/21f8e7b001337051a0bfa84c30f53ce557c99d1b/components/offline_pages/core/offline_page_model_impl.h
►

Sign in to add a comment