Issue 622763 link

Starred by 2 users

Issue metadata

Status:	Fixed
Owner:	dewittj@chromium.org
Closed:	Nov 2016
Cc:	█ dim...@chromium.org fgor...@chromium.org
Components:	UI>Browser>Offline
EstimatedDays:	----
NextAction:	----
OS:	Android
Pri:	2
Type:	Bug

Blocked on:
issue 622424

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.

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

Comment 1 by dewittj@chromium.org, Sep 14 2016

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

Comment 4 by dewittj@chromium.org, Sep 23 2016

Status: Started (was: Assigned)

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 27 2016

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

commit e71c771311d6350f0264b3aa47c1d3aaabdd4492
Author: dewittj <dewittj@chromium.org>
Date: Tue Sep 27 15:59:27 2016

Offline Pages - Removes the "ClearAllPages" API.

This API is unused, cleaning up offline page model.

BUG= 622763 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/e71c771311d6350f0264b3aa47c1d3aaabdd4492/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/e71c771311d6350f0264b3aa47c1d3aaabdd4492/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/e71c771311d6350f0264b3aa47c1d3aaabdd4492/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/e71c771311d6350f0264b3aa47c1d3aaabdd4492/components/offline_pages/offline_page_model_impl_unittest.cc
[modify] https://crrev.com/e71c771311d6350f0264b3aa47c1d3aaabdd4492/components/offline_pages/stub_offline_page_model.cc
[modify] https://crrev.com/e71c771311d6350f0264b3aa47c1d3aaabdd4492/components/offline_pages/stub_offline_page_model.h

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 7 2016

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

commit 7be8aa05df5cdfc13cb1b68dbbd9710080ff2059
Author: dewittj <dewittj@chromium.org>
Date: Fri Oct 07 18:19:14 2016

[Offline Pages] Adds new policy bits and reverse lookup.

This is part of the query API project to support looking up offline
pages by feature (which may contain multiple policies).  Also removes
the namespace check found in OfflinePageUtils.

BUG= 622763 

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

[modify] https://crrev.com/7be8aa05df5cdfc13cb1b68dbbd9710080ff2059/chrome/browser/android/offline_pages/offline_page_utils.cc
[modify] https://crrev.com/7be8aa05df5cdfc13cb1b68dbbd9710080ff2059/components/offline_pages/client_policy_controller.cc
[modify] https://crrev.com/7be8aa05df5cdfc13cb1b68dbbd9710080ff2059/components/offline_pages/client_policy_controller.h
[modify] https://crrev.com/7be8aa05df5cdfc13cb1b68dbbd9710080ff2059/components/offline_pages/client_policy_controller_unittest.cc
[modify] https://crrev.com/7be8aa05df5cdfc13cb1b68dbbd9710080ff2059/components/offline_pages/offline_page_client_policy.h

Comment 7 by dewittj@chromium.org, Oct 20 2016

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

Comment 10 by dewittj@chromium.org, Nov 8 2016

Status: Fixed (was: Started)

►

Issue 622763 link

Issue metadata

Implement a proper query interface for the offline page model.

Issue description

Comment 1 by dewittj@chromium.org, Sep 14 2016

Comment 1 Deleted

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

Comment 2 Deleted

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

Comment 3 Deleted

Comment 4 by dewittj@chromium.org, Sep 23 2016

Comment 4 Deleted

Comment 5 by bugdroid1@chromium.org, Sep 27 2016

Comment 5 Deleted

Comment 6 by bugdroid1@chromium.org, Oct 7 2016

Comment 6 Deleted

Comment 7 by dewittj@chromium.org, Oct 20 2016

Comment 7 Deleted

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

Comment 8 Deleted

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

Comment 9 Deleted

Comment 10 by dewittj@chromium.org, Nov 8 2016

Comment 10 Deleted

Sign in to add a comment