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

Issue 615165 link

Starred by 1 user
Status: Duplicate
Merged: issue 610824
Owner:
dougarnett@chromium.org
Closed: Jun 2016
Cc:
dewittj@chromium.org
█ dim...@chromium.org
fgor...@chromium.org
dougarnett@chromium.org
█ romax@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

OfflinePageModel cleanups

Project Member Reported by dougarnett@chromium.org, May 26 2016 
In splitting the OfflinePageModel into a separate interface and implementation, these cleanup ideas were discovered:

- Relocate and rename CanSavePage() static method to better home (Utils?)
- Better home for kBookmarkNamespace and kInvalidOfflineId?
- Get rid of is_loaded() and bridge usage of it.
- Put OfflinePageStorageManager::Client subinterface back into OfflinePageModel
- Move *Result definitions from offline_page_types.h to new OfflinePageModel pure interface.

Comment 1 by fgor...@chromium.org, May 27 2016 

Duplicate of 610824?
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 2 2016 

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

commit 128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f
Author: dougarnett <dougarnett@chromium.org>
Date: Thu Jun 02 21:52:01 2016

OfflinePageModel cleanups - improved CanSavePage() and moved static kInvalidOfflineId to impl.

[Tried to move CanSavePage to OfflinePageUtils but still want to use in components. In the process discovered other cleanup to do on that method.]

BUG= 615165 

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

[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/components/offline_pages/offline_page_model.cc
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/128b7c0fff1d970e8c37e2a82b8cb4a168ba9d7f/components/offline_pages/offline_page_model_impl_unittest.cc

Comment 3 by dougarnett@chromium.org, Jun 2 2016

Mergedinto: 610824
Status: Duplicate (was: Untriaged)
Filip has doc in progress that should cover these cleanup ideas and many more so duping this into his broader bug.
►

Sign in to add a comment