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

Issue 622763 link

Starred by 2 users

Issue metadata

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

Blocked on:
issue 622424



Sign in to add a comment

Implement a proper query interface for the offline page model.

Project Member Reported by dewittj@chromium.org, Jun 23 2016

Issue description

To support a faster timeline, we are adding a quick check to prevent offline pages from being shown on the new tab page that don't belong there.

This bug tracks removing that quick check and properly updating the API to support the types of querying we need.

Design doc (sorry, internal only) 
https://docs.google.com/document/d/1Uoi7zZNs_Exs30noRL2OvlP5cI8qxA9B3uAqOF_ysGU/edit?disco=AAAAAxO9wSM


 
Summary: Implement a proper query interface for the offline page model. (was: Replace hacky "Last-N" check with a proper query interface for N==1)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 14 2016

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

commit ed3f594eb4755ca8818de9b1e015f6caab5656b1
Author: dewittj <dewittj@chromium.org>
Date: Wed Sep 14 16:11:51 2016

[Offline Pages] Removes the unused "HasPages" function.

This is part of a cleanup before switching to a query interface.

BUG= 622763 

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

[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/chrome/browser/android/offline_pages/offline_page_bridge.h
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/components/offline_pages/offline_page_model_impl_unittest.cc
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/components/offline_pages/stub_offline_page_model.cc
[modify] https://crrev.com/ed3f594eb4755ca8818de9b1e015f6caab5656b1/components/offline_pages/stub_offline_page_model.h

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 23 2016

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

commit e33be0330905a74f956c8563fee470cafc47ce7b
Author: dewittj <dewittj@chromium.org>
Date: Fri Sep 23 19:44:04 2016

Offline Pages: Removes the metadata consistency check from the model API.

This check is implementation-dependent (and unused outside the model).
As such it belongs only inside the offline page model.

BUG= 622763 

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

[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/chrome/browser/android/offline_pages/offline_page_bridge.h
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/components/offline_pages/offline_page_model_impl_unittest.cc
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/components/offline_pages/stub_offline_page_model.cc
[modify] https://crrev.com/e33be0330905a74f956c8563fee470cafc47ce7b/components/offline_pages/stub_offline_page_model.h

Status: Started (was: Assigned)
Description: Show this description
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 21 2016

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

commit 4af0fc42ddb3a671b7e71541fe3a7815de26a0a1
Author: dewittj <dewittj@chromium.org>
Date: Fri Oct 21 16:08:39 2016

Remove all synchronous methods from OfflinePageBridge.

This blocks adding features to the query interface, which is asynchronous by design.

BUG= 589526 , 622763 

Review-Url: https://chromiumcodereview.appspot.com/2429943002
Cr-Commit-Position: refs/heads/master@{#426805}

[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridge.java
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/chrome/android/junit/src/org/chromium/chrome/browser/offlinepages/OfflinePageBridgeUnitTest.java
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/chrome/browser/android/offline_pages/offline_page_bridge.h
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/components/offline_pages/offline_page_model_impl_unittest.cc
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/components/offline_pages/stub_offline_page_model.cc
[modify] https://crrev.com/4af0fc42ddb3a671b7e71541fe3a7815de26a0a1/components/offline_pages/stub_offline_page_model.h

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 28 2016

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

commit cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215
Author: dewittj <dewittj@chromium.org>
Date: Fri Oct 28 17:01:08 2016

Query API: Introduces an OfflinePageModelQuery object.

Replaces most query operations within OfflinePageModelImpl with query-based versions.

BUG= 622763 

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

[modify] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/BUILD.gn
[modify] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/client_policy_controller.cc
[modify] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/client_policy_controller.h
[modify] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/offline_page_model_impl_unittest.cc
[add] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/offline_page_model_query.cc
[add] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/offline_page_model_query.h
[add] https://crrev.com/cc2f7d0c5c3ab42762d9e88ef1f4b293b03b6215/components/offline_pages/offline_page_model_query_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment