Support resaving offline page right after it is being deleted

Support resaving offline page right after it is being deleted

BUG=597398
Committed: https://crrev.com/661a94d42b5131ea7d2770c445131c39c6740eb6
Cr-Commit-Position: refs/heads/master@{#384151}

[jianli, 2016-03-30 00:37:26]

jianli@chromium.org changed reviewers:
+ fgorski@chromium.org

[fgorski, 2016-03-30 01:43:30]

lgtm, but consider dropping url_hash and fix compilation problems.

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/offline_page_mhtml_archiver.cc
(right):

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/offline_page_mhtml_archiver.cc:43:
base::Base64Encode(url_hash, &url_hash);
if we are adding offline_id here, do we still need a url_hash?

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h (right):

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h:52: int64_t
id);
nit: be consistent with naming, especially if this is archive_id

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h:69: int64_t
archive_id);
Seems like there is a compilation error here somewhere.

https://codereview.chromium.org/1843893002/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model_unittest.cc (right):

https://codereview.chromium.org/1843893002/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model_unittest.cc:491:
TEST_F(OfflinePageModelTest, ResavePageAfterMarkingPageForDeletion) {
SavePageAfter...

[jianli, 2016-03-30 23:16:26]

The CQ bit was checked by jianli@chromium.org

[jianli, 2016-03-30 23:16:26]

The patchset sent to the CQ was uploaded after l-g-t-m from fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/1843893002/#ps20001
(title: "Address feedback")

[commit-bot: I haz the power, 2016-03-30 23:17:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1843893002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1843893002/20001

[commit-bot: I haz the power, 2016-03-31 00:06:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-31 00:08:03]

Description was changed from

==========
Support resaving offline page right after it is being deleted

BUG=597398
==========

to

==========
Support resaving offline page right after it is being deleted

BUG=597398

Committed: https://crrev.com/661a94d42b5131ea7d2770c445131c39c6740eb6
Cr-Commit-Position: refs/heads/master@{#384151}
==========

[commit-bot: I haz the power, 2016-03-31 00:08:04]

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

[jianli, 2016-03-31 21:02:43]

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/offline_page_mhtml_archiver.cc
(right):

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/offline_page_mhtml_archiver.cc:43:
base::Base64Encode(url_hash, &url_hash);
On 2016/03/30 01:43:29, fgorski wrote:
> if we are adding offline_id here, do we still need a url_hash?

Done.

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
File chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h (right):

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h:52: int64_t
id);
On 2016/03/30 01:43:30, fgorski wrote:
> nit: be consistent with naming, especially if this is archive_id

Done.

https://codereview.chromium.org/1843893002/diff/1/chrome/browser/android/offl...
chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h:69: int64_t
archive_id);
On 2016/03/30 01:43:30, fgorski wrote:
> Seems like there is a compilation error here somewhere.

Done.

https://codereview.chromium.org/1843893002/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_model_unittest.cc (right):

https://codereview.chromium.org/1843893002/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_model_unittest.cc:491:
TEST_F(OfflinePageModelTest, ResavePageAfterMarkingPageForDeletion) {
On 2016/03/30 01:43:30, fgorski wrote:
> SavePageAfter...

Done.