Create skeleton for the Prefetching store and initial pipeline step.

Create skeleton for the Prefetching store and initial pipeline step.

Add the groundwork of the  Prefetching Offline Pages persistent store and what
will become the first step of the URL pipeline, that accepts URL suggestions
from Zine.

BUG=701939
Review-Url: https://codereview.chromium.org/2879013002
Cr-Commit-Position: refs/heads/master@{#476855}
Committed: https://chromium.googlesource.com/chromium/src/+/76848ad61df944e68cda21e4fbc5e2565bd80f08

[carlosk, 2017-05-13 02:16:45]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-13 02:17:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-13 02:55:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-13 02:55:11]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[carlosk, 2017-05-16 01:35:20]

Description was changed from

==========
Create the Zine suggestions step of Prefetching Offline Pages.

Add the first step of the Prefetching Offline Pages pipeline.

BUG=701939
==========

to

==========
Create the skeleton of the Zine suggestions pipeline step.

Add the groundwork of the first step of the Prefetching Offline Pages pipeline
which accepts URL suggestions from Zine to be prefetched.

BUG=701939
==========

[carlosk, 2017-05-16 01:35:40]

Description was changed from

==========
Create the skeleton of the Zine suggestions pipeline step.

Add the groundwork of the first step of the Prefetching Offline Pages pipeline
which accepts URL suggestions from Zine to be prefetched.

BUG=701939
==========

to

==========
Create the skeleton of the Zine suggestions pipeline step.

Add the groundwork of the first step of the Prefetching Offline Pages pipeline
which accepts URL suggestions from Zine to be prefetched.

BUG=701939
==========

[carlosk, 2017-05-17 23:01:09]

Description was changed from

==========
Create the skeleton of the Zine suggestions pipeline step.

Add the groundwork of the first step of the Prefetching Offline Pages pipeline
which accepts URL suggestions from Zine to be prefetched.

BUG=701939
==========

to

==========
Create skeleton for the Prefetching store and initial pipeline step.

Add the groundwork of the  Prefetching Offline Pages persistent store and what
will become the first step of the URL pipeline, that accepts URL suggestions
from Zine.

BUG=701939
==========

[carlosk, 2017-05-17 23:01:19]

Description was changed from

==========
Create skeleton for the Prefetching store and initial pipeline step.

Add the groundwork of the  Prefetching Offline Pages persistent store and what
will become the first step of the URL pipeline, that accepts URL suggestions
from Zine.

BUG=701939
==========

to

==========
Create skeleton for the Prefetching store and initial pipeline step.

Add the groundwork of the  Prefetching Offline Pages persistent store and what
will become the first step of the URL pipeline, that accepts URL suggestions
from Zine.

BUG=701939
==========

[carlosk, 2017-05-17 23:01:36]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-17 23:03:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[carlosk, 2017-05-17 23:33:35]

carlosk@chromium.org changed reviewers:
+ dewittj@chromium.org, dimich@chromium.org

[carlosk, 2017-05-17 23:33:35]

dimich@, dewittj@: PTAL.

jianli@: FYI.

[carlosk, 2017-05-17 23:34:25]

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

[carlosk, 2017-05-17 23:34:26]

fgorski@: PTAL (because of initial store definition).

[Dmitry Titov, 2017-05-18 21:24:02]

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/content/prefetch_service_factory.cc (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/content/prefetch_service_factory.cc:38:
std::unique_ptr<PrefetchStore> prefetch_store;
Does this create an instance of abstract class? Do you want to add a
PrefetchStoreImpl with trivial implementaiton and add it here?

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.h:22: // any of the
suggestions for the same namespace are deleted from the store.
Could you please add a sentence here explaining to a casual reader why this
specific treatment of ZOMBIES?

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h:35:
std::unique_ptr<PrefetchStore> store_;
Should these rather live on PrefetchService and be injected in its constructor?
The PrefetchDispatcher may just keep a pointer back to PrefetchService to
retrieve other subcomponents.

Sooner or later it will have such a pointer and will pull other subcomponents
form the Service, so it might be good to just adopt this pattern now.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_store.h (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_store.h:31: // Resets the store.
What exactly "Resets" mean? Could you add more to the comment?

[dewittj, 2017-05-23 20:02:20]

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:20: void
AddUniqueUrlsTask::Run() {
Task::TaskComplete is never called?

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:23:
PrefetchStore::CreateItemsForUniqueUrlsCallback());
This just runs a void callback, doesn't make sense to me.   Seems that the
callback should be passed in the constructor?

[carlosk, 2017-05-24 20:55:09]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-24 20:56:02]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[carlosk, 2017-05-24 20:57:26]

Thanks. I also made the dispatcher be an injected dependency of the service as
discussed offline.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/content/prefetch_service_factory.cc (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/content/prefetch_service_factory.cc:38:
std::unique_ptr<PrefetchStore> prefetch_store;
On 2017/05/18 21:24:02, Dmitry Titov wrote:
> Does this create an instance of abstract class? Do you want to add a
> PrefetchStoreImpl with trivial implementaiton and add it here?

This stores a null pointer. And I like the idea of getting clean null crashes
while this is not implemented. Adding the base implementation is part of the
followup step.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:20: void
AddUniqueUrlsTask::Run() {
On 2017/05/23 20:02:20, dewittj wrote:
> Task::TaskComplete is never called?

Done.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:23:
PrefetchStore::CreateItemsForUniqueUrlsCallback());
On 2017/05/23 20:02:20, dewittj wrote:
> This just runs a void callback, doesn't make sense to me.   Seems that the
> callback should be passed in the constructor?

Now that we need to call TaskComplete we need the callback and it should be
controlled by the task. Also this task is expected to have more steps so the
callback will eventually trigger followup work.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.h:22: // any of the
suggestions for the same namespace are deleted from the store.
On 2017/05/18 21:24:02, Dmitry Titov wrote:
> Could you please add a sentence here explaining to a casual reader why this
> specific treatment of ZOMBIES?

Done.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_dispatcher_impl.h:35:
std::unique_ptr<PrefetchStore> store_;
On 2017/05/18 21:24:02, Dmitry Titov wrote:
> Should these rather live on PrefetchService and be injected in its
constructor?
> The PrefetchDispatcher may just keep a pointer back to PrefetchService to
> retrieve other subcomponents.
> 
> Sooner or later it will have such a pointer and will pull other subcomponents
> form the Service, so it might be good to just adopt this pattern now.
> 

Done.

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_store.h (right):

https://codereview.chromium.org/2879013002/diff/60001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_store.h:31: // Resets the store.
On 2017/05/18 21:24:02, Dmitry Titov wrote:
> What exactly "Resets" mean? Could you add more to the comment?

Even better: I'll fully remove it for now as it's not being used. :)

[dewittj, 2017-05-24 21:09:19]

lgtm

[commit-bot: I haz the power, 2017-05-24 22:33:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-24 22:33:49]

Dry run: This issue passed the CQ dry run.

[Dmitry Titov, 2017-05-26 23:05:06]

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:23:
store_->CreateItemsForUniqueUrls(name_space_, prefetch_urls_,
This feels like delegating the whole thing to Store... Perhaps PrefetchStore
should be less aware about semantics of this feature and give a more sql-ish
(CRUD) interface. So in this case there would be store->ReadItems(sql_string),
check for ZOMBIES etc, then store->AddItems(items) and mayeb
store->UpdateItems(vector<item>)

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.h:38: PrefetchStore*
store_;
Naming: I think it should be prefetch_store_.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.h:21: virtual
PrefetchStore* GetStore() = 0;
I think it would be more consistent to either have

virtual Store* GetStore();  // this likely needs a new namespace
or
virtual PrefetchStore* GetPrefetchStore()

same for GetDispatcher above...

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service_impl.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service_impl.h:29:
std::unique_ptr<PrefetchStore> store_;
prefetch_store_

[fgorski, 2017-05-30 17:21:46]

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/content/prefetch_service_factory.cc (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/content/prefetch_service_factory.cc:40:
std::unique_ptr<PrefetchDispatcher> prefetch_dispatcher_impl =
nit: prefetch_dispatcher would be suitable I guess.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.h:25: // step
described above is done, all same namespace items in the ZOMBIE state
Zombie clean up could be a separate task that is scheduled right after adding
URLs.

Of course does not need to be split now it they are always done together.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc:38: const
std::string& name_space,
why are you making this change? (I think I lack context)

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.h:20: virtual
PrefetchDispatcher* GetDispatcher() = 0;
Why are these public?
How do you intend to use the service?

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_types.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_types.h:118: std::string
client_id;
You are overloading the term Client ID.
Is that intended? and what is the thinking behind it?

[carlosk, 2017-06-01 01:44:18]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 01:45:00]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 01:48:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-01 01:48:37]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[carlosk, 2017-06-01 01:49:59]

Thanks.

IRT to method/variable naming: I was trying to avoid having "prefetch" mentioned
everywhere. To address some comments and added a more of them so LMKWDYT.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/content/prefetch_service_factory.cc (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/content/prefetch_service_factory.cc:40:
std::unique_ptr<PrefetchDispatcher> prefetch_dispatcher_impl =
On 2017/05/30 17:21:45, fgorski wrote:
> nit: prefetch_dispatcher would be suitable I guess.

As here it is specifically an instance of the implementation I'd prefer to keep
the impl part.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:23:
store_->CreateItemsForUniqueUrls(name_space_, prefetch_urls_,
On 2017/05/26 23:05:06, Dmitry Titov wrote:
> This feels like delegating the whole thing to Store... Perhaps PrefetchStore
> should be less aware about semantics of this feature and give a more sql-ish
> (CRUD) interface. So in this case there would be store->ReadItems(sql_string),
> check for ZOMBIES etc, then store->AddItems(items) and mayeb
> store->UpdateItems(vector<item>)

From my talks with fgorski@ I think the store and task design will change
considerably from was represented here. But also this patch has already grown
large enough so I fully removed the store implementation for it, punting it to a
followup patch.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.h:25: // step
described above is done, all same namespace items in the ZOMBIE state
On 2017/05/30 17:21:45, fgorski wrote:
> Zombie clean up could be a separate task that is scheduled right after adding
> URLs.
> 
> Of course does not need to be split now it they are always done together.

With the current design of using zombies as a duplicate entry detector their
cleanup must be a follow up task of adding new entries.

If at some point we should need to run that cleanup at another pipelie step it
will be split out to another task.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/add_unique_urls_task.h:38: PrefetchStore*
store_;
On 2017/05/26 23:05:06, Dmitry Titov wrote:
> Naming: I think it should be prefetch_store_. 

Done.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_dispatcher_impl.cc:38: const
std::string& name_space,
On 2017/05/30 17:21:45, fgorski wrote:
> why are you making this change? (I think I lack context)

Reverted to using a generalized version of ClientId, now that it has been moved
out of OfflinePageItem and into its own file.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.h:20: virtual
PrefetchDispatcher* GetDispatcher() = 0;
On 2017/05/30 17:21:45, fgorski wrote:
> Why are these public?
> How do you intend to use the service?

The service is supposed to be the ownership holder of all prefetching
sub-components, hence this getter. But I see where your doubt comes from and how
this could be influenced by the design of the store.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service.h:21: virtual
PrefetchStore* GetStore() = 0;
On 2017/05/26 23:05:06, Dmitry Titov wrote:
> I think it would be more consistent to either have
> 
> virtual Store* GetStore();  // this likely needs a new namespace
> or
> virtual PrefetchStore* GetPrefetchStore()
> 
> same for GetDispatcher above...

Done.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_service_impl.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_service_impl.h:29:
std::unique_ptr<PrefetchStore> store_;
On 2017/05/26 23:05:06, Dmitry Titov wrote:
> prefetch_store_

Done, and below too.

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
File components/offline_pages/core/prefetch/prefetch_types.h (right):

https://codereview.chromium.org/2879013002/diff/80001/components/offline_page...
components/offline_pages/core/prefetch/prefetch_types.h:118: std::string
client_id;
On 2017/05/30 17:21:45, fgorski wrote:
> You are overloading the term Client ID.
> Is that intended? and what is the thinking behind it? 

It would have the same objective as the existing ClientId, hence the very
similar naming which was indeed overloading that concept with only a part of it.

With the revert to using ClientId I expect this will not be a concern anymore.

[carlosk, 2017-06-01 20:21:14]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 20:21:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 20:33:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-01 20:33:05]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[carlosk, 2017-06-01 21:01:29]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 21:01:52]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dmitry Titov, 2017-06-02 00:01:48]

lgtm!

One nit:

https://codereview.chromium.org/2879013002/diff/180001/components/offline_pag...
File components/offline_pages/core/prefetch/prefetch_service.h (right):

https://codereview.chromium.org/2879013002/diff/180001/components/offline_pag...
components/offline_pages/core/prefetch/prefetch_service.h:31: virtual
PrefetchDispatcher* GetPrefetchDispatcher() = 0;
Lets alphabetize the order of these GetXYZ methods...

[commit-bot: I haz the power, 2017-06-02 01:26:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-02 01:26:16]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[carlosk, 2017-06-02 01:59:57]

Thanks.

https://codereview.chromium.org/2879013002/diff/180001/components/offline_pag...
File components/offline_pages/core/prefetch/prefetch_service.h (right):

https://codereview.chromium.org/2879013002/diff/180001/components/offline_pag...
components/offline_pages/core/prefetch/prefetch_service.h:31: virtual
PrefetchDispatcher* GetPrefetchDispatcher() = 0;
On 2017/06/02 00:01:48, Dmitry Titov wrote:
> Lets alphabetize the order of these GetXYZ methods...

Done.

[carlosk, 2017-06-02 02:39:49]

The CQ bit was checked by carlosk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 02:40:11]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-02 03:59:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-02 04:00:00]

Dry run: This issue passed the CQ dry run.

[fgorski, 2017-06-02 17:33:32]

lgtm with a comment

https://codereview.chromium.org/2879013002/diff/220001/components/offline_pag...
File components/offline_pages/core/client_id.h (right):

https://codereview.chromium.org/2879013002/diff/220001/components/offline_pag...
components/offline_pages/core/client_id.h:18: ClientId(std::string name_space,
std::string id);
Since you are moving stuff, these can be const&s.

[carlosk, 2017-06-02 17:51:26]

Thanks.

https://codereview.chromium.org/2879013002/diff/220001/components/offline_pag...
File components/offline_pages/core/client_id.h (right):

https://codereview.chromium.org/2879013002/diff/220001/components/offline_pag...
components/offline_pages/core/client_id.h:18: ClientId(std::string name_space,
std::string id);
On 2017/06/02 at 17:33:31, fgorski wrote:
> Since you are moving stuff, these can be const&s.

Done.

[carlosk, 2017-06-02 17:54:07]

The CQ bit was checked by carlosk@chromium.org

[carlosk, 2017-06-02 17:54:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
dewittj@chromium.org, dimich@chromium.org, fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2879013002/#ps240001
(title: "Made construction params a cost&.")

[commit-bot: I haz the power, 2017-06-02 17:54:38]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-02 22:09:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-02 22:09:24]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dewittj, 2017-06-02 22:14:21]

The CQ bit was checked by dewittj@chromium.org

[commit-bot: I haz the power, 2017-06-02 22:14:54]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-03 01:58:45]

CQ is committing da patch.

Bot data: {"patchset_id": 240001, "attempt_start_ts": 1496441661222430,
"parent_rev": "657ba4ddf2ab939ce918666bcb89328e9eae93bc", "commit_rev":
"76848ad61df944e68cda21e4fbc5e2565bd80f08"}

[commit-bot: I haz the power, 2017-06-03 01:58:54]

Description was changed from

==========
Create skeleton for the Prefetching store and initial pipeline step.

Add the groundwork of the  Prefetching Offline Pages persistent store and what
will become the first step of the URL pipeline, that accepts URL suggestions
from Zine.

BUG=701939
==========

to

==========
Create skeleton for the Prefetching store and initial pipeline step.

Add the groundwork of the  Prefetching Offline Pages persistent store and what
will become the first step of the URL pipeline, that accepts URL suggestions
from Zine.

BUG=701939

Review-Url: https://codereview.chromium.org/2879013002
Cr-Commit-Position: refs/heads/master@{#476855}
Committed:
https://chromium.googlesource.com/chromium/src/+/76848ad61df944e68cda21e4fbc5...
==========

[commit-bot: I haz the power, 2017-06-03 01:58:56]

Committed patchset #13 (id:240001) as
https://chromium.googlesource.com/chromium/src/+/76848ad61df944e68cda21e4fbc5...