Prefetching: Introduce store commands abstractions to be used by tasks.

Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

BUG=701939
Review-Url: https://codereview.chromium.org/2920083002
Cr-Commit-Position: refs/heads/master@{#480292}
Committed: https://chromium.googlesource.com/chromium/src/+/12e48fc0602a0703d598c51577dbe918a3295914

[carlosk, 2017-06-03 01:13:08]

Description was changed from

==========
Prefetching: Zine related tasks implementation.

Create the tasks needed by API offered to Zine for prefetching offline pagets.
Also defines the initial architecture for store commands but without touching
SQL yet.

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. The prefetch service centralizes ownership of subcomponents.

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

to

==========
Prefetching: Zine related tasks implementation.

Create the tasks needed by API offered to Zine for prefetching offline pagets.
Also defines the initial architecture for store commands but without touching
SQL yet.

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. The prefetch service centralizes ownership of subcomponents.

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

[carlosk, 2017-06-10 02:49:11]

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

[commit-bot: I haz the power, 2017-06-10 02:49:26]

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-10 02:58:43]

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

[commit-bot: I haz the power, 2017-06-10 02:58:46]

Dry run: Try jobs failed on following builders:
  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-13 00:55:01]

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

[commit-bot: I haz the power, 2017-06-13 00:55:24]

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-13 00:58:04]

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

[commit-bot: I haz the power, 2017-06-13 00:58:05]

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-...)
  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-13 00:59:16]

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

[carlosk, 2017-06-13 00:59:17]

fgorski@: PTAL.

dimich@, dewittj@: FYI.

(obviously accepting comments from everyone but just wanted to assign at least
one formal reviewer from the start)

In an offline discussion dewittj@ already voiced some concerns and suggestions
that seem reasonable and will help simplify this approach to StoreCommand. We
can (should?) discuss them further but I'll work on them for the followup patch
set.

[dewittj, 2017-06-13 18:02:04]

dewittj@chromium.org changed reviewers:
+ dewittj@chromium.org

[dewittj, 2017-06-13 18:02:06]

Some of this we already discussed but I wanted to codify in my code review for
fgorski/dimich's benefit.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/add_unique_urls_task.h:30:
AddUniqueUrlsTask(
this looks really nice!

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File components/offline_pages/core/prefetch/store/add_urls_store_command.h
(right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/add_urls_store_command.h:22:
virtual void Configure(const std::vector<PrefetchURL>& prefetch_urls) = 0;
I'd rather this be more specific.  Something like:

SetUrlsToAdd(auto prefetch_urls) = 0;

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File
components/offline_pages/core/prefetch/store/cleanup_zombies_store_command.h
(right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/cleanup_zombies_store_command.h:18:
virtual void Configure(const std::vector<PrefetchURL>& prefetch_urls) = 0;
I'd rather this be more specific.  Something like:

// These URLs will not be removed as zombies so we can prevent duplicate work.
SetActivePrefetchURLs(const std::vector...) = 0;

makes more sense than a generic Configure, since it's self-documenting.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File components/offline_pages/core/prefetch/store/store_command.h (right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/store_command.h:21: class
StoreCommand {
As we discussed, this interface is entirely about convention.  There is no
object that takes generic StoreCommands and calls Execute on them.  I prefer not
to have code like this in the system, instead we can have a pattern that is
followed.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/store_command.h:25: virtual void
Execute(base::OnceCallback<T> callback) = 0;
To me, it feels easier to support complex return values if the command itself
holds the result data, and the callback for Execute is simply a void().  Then
the caller can interrogate the command itself for the potentially multiple
return values.

Otherwise we'll start having std::tuples or complex objects in the callback. 
Not sure if we need/want that.

[carlosk, 2017-06-13 23:22:53]

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

[commit-bot: I haz the power, 2017-06-13 23:23:27]

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-14 00:34:00]

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

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

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-14 20:08:42]

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

[commit-bot: I haz the power, 2017-06-14 20:09:07]

Dry run: CQ is trying da patch.

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

[carlosk, 2017-06-14 20:30:48]

Description was changed from

==========
Prefetching: Zine related tasks implementation.

Create the tasks needed by API offered to Zine for prefetching offline pagets.
Also defines the initial architecture for store commands but without touching
SQL yet.

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. The prefetch service centralizes ownership of subcomponents.

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

to

==========
Prefetching: Introduce commands abstraction into the persistent store.

Updates the AddUniqueUrlsTask (still empty of actual business logic) to use
store commands as an abstraction layer to execute operations on the persistent
prefetching store. The command and store actual implementations are not yet
provided.

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

[carlosk, 2017-06-14 20:31:05]

Description was changed from

==========
Prefetching: Introduce commands abstraction into the persistent store.

Updates the AddUniqueUrlsTask (still empty of actual business logic) to use
store commands as an abstraction layer to execute operations on the persistent
prefetching store. The command and store actual implementations are not yet
provided.

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

to

==========
Prefetching: Introduce commands abstraction into the persistent store.

Updates the AddUniqueUrlsTask (still empty of actual business logic) to use
store commands as an abstraction layer to execute operations on the persistent
prefetching store. The command and store actual implementations are not yet
provided.

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

[carlosk, 2017-06-14 20:32:47]

Thanks.

fgorski@: Could you PTAL at dewittj@'s suggestion of eliminating StoreCommand?

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File components/offline_pages/core/prefetch/add_unique_urls_task.h (right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/add_unique_urls_task.h:30:
AddUniqueUrlsTask(
On 2017/06/13 18:02:05, dewittj wrote:
> this looks really nice!

Acknowledged and thanks.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File components/offline_pages/core/prefetch/store/add_urls_store_command.h
(right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/add_urls_store_command.h:22:
virtual void Configure(const std::vector<PrefetchURL>& prefetch_urls) = 0;
On 2017/06/13 18:02:05, dewittj wrote:
> I'd rather this be more specific.  Something like:
> 
> SetUrlsToAdd(auto prefetch_urls) = 0;

Done.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File
components/offline_pages/core/prefetch/store/cleanup_zombies_store_command.h
(right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/cleanup_zombies_store_command.h:18:
virtual void Configure(const std::vector<PrefetchURL>& prefetch_urls) = 0;
On 2017/06/13 18:02:05, dewittj wrote:
> I'd rather this be more specific.  Something like:
> 
> // These URLs will not be removed as zombies so we can prevent duplicate work.
> SetActivePrefetchURLs(const std::vector...) = 0;
> 
> makes more sense than a generic Configure, since it's self-documenting.

Done.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
File components/offline_pages/core/prefetch/store/store_command.h (right):

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/store_command.h:21: class
StoreCommand {
On 2017/06/13 18:02:05, dewittj wrote:
> As we discussed, this interface is entirely about convention.  There is no
> object that takes generic StoreCommands and calls Execute on them.  I prefer
not
> to have code like this in the system, instead we can have a pattern that is
> followed.

Yes, I begin to agree with this: eliminating StoreCommand has (currently, AFAIK)
no architectural impact and would (re-)allow for Execute callbacks to be more
elaborate (like re-accepting a simple `bool success` parameter). But I'd like to
have fgorski@ opinion on this before moving on with fully removing it.

https://codereview.chromium.org/2920083002/diff/100001/components/offline_pag...
components/offline_pages/core/prefetch/store/store_command.h:25: virtual void
Execute(base::OnceCallback<T> callback) = 0;
On 2017/06/13 18:02:05, dewittj wrote:
> To me, it feels easier to support complex return values if the command itself
> holds the result data, and the callback for Execute is simply a void().  Then
> the caller can interrogate the command itself for the potentially multiple
> return values.

I agree and switched to that.

> Otherwise we'll start having std::tuples or complex objects in the callback. 
> Not sure if we need/want that.

As base::Callback can receive an arbitrary list of parameters I don't see the
how we would get to needing complex structures. But I agree commands could offer
specialized result getters if one would return a long list of isolated results.

https://codereview.chromium.org/2920083002/diff/140001/components/offline_pag...
File components/offline_pages/core/prefetch/store/add_urls_store_command.h
(right):

https://codereview.chromium.org/2920083002/diff/140001/components/offline_pag...
components/offline_pages/core/prefetch/store/add_urls_store_command.h:36:
DISALLOW_COPY_AND_ASSIGN(AddUrlsStoreCommand);
I'm still a bit unsure of where in the command class hierarchy this should be.

[carlosk, 2017-06-15 20:53:25]

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

[commit-bot: I haz the power, 2017-06-15 20:54:15]

Dry run: CQ is trying da patch.

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

[carlosk, 2017-06-15 22:29:08]

This latest patch set conforms with the latest design discussion with fgorski@.

Removing StoreCommand made everything simpler, including test implementations.
And we kept Tasks still isolated from SQL code.

Also AddUniqueUrlsTask is already the final implementation of the main Zine task
and the 1st step of our pipeline (so I should probably update the description
again to mention that). It is very simple and that makes me happy. :)

PTAL!

https://codereview.chromium.org/2920083002/diff/200001/components/offline_pag...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2920083002/diff/200001/components/offline_pag...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:36: // the store.
I'll leave this as a TODO right now as it will probably require creating a new
abstraction.

[carlosk, 2017-06-15 22:35:01]

Description was changed from

==========
Prefetching: Introduce commands abstraction into the persistent store.

Updates the AddUniqueUrlsTask (still empty of actual business logic) to use
store commands as an abstraction layer to execute operations on the persistent
prefetching store. The command and store actual implementations are not yet
provided.

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

to

==========
Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

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

[carlosk, 2017-06-15 22:35:14]

Description was changed from

==========
Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

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

to

==========
Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

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

[dewittj, 2017-06-15 22:46:50]

lgtm, thanks

https://codereview.chromium.org/2920083002/diff/200001/chrome/browser/offline...
File chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc (right):

https://codereview.chromium.org/2920083002/diff/200001/chrome/browser/offline...
chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc:50: // TODO:
Instantiate an actual factory once one exists.
nit: This makes prefetching crash if enabled.  Not an ideal state of affairs for
very long.

https://codereview.chromium.org/2920083002/diff/200001/components/offline_pag...
File components/offline_pages/core/prefetch/prefetch_service_test_taco.cc
(right):

https://codereview.chromium.org/2920083002/diff/200001/components/offline_pag...
components/offline_pages/core/prefetch/prefetch_service_test_taco.cc:25: //
TODO(carlosk): initialize store_command_factory_ once a test implementation
Isn't there a test one now?

[carlosk, 2017-06-16 00: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-16 00:44:36]

Dry run: CQ is trying da patch.

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

[carlosk, 2017-06-16 01:06:58]

Thanks.

https://codereview.chromium.org/2920083002/diff/200001/chrome/browser/offline...
File chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc (right):

https://codereview.chromium.org/2920083002/diff/200001/chrome/browser/offline...
chrome/browser/offline_pages/prefetch/prefetch_service_factory.cc:50: // TODO:
Instantiate an actual factory once one exists.
On 2017/06/15 22:46:49, dewittj wrote:
> nit: This makes prefetching crash if enabled.  Not an ideal state of affairs
for
> very long.

Is it OK to leave it like that or would you prefer that I use the (just
extracted) test version instead? Note that that would require also including
those test files in the non-test build target.

https://codereview.chromium.org/2920083002/diff/200001/components/offline_pag...
File components/offline_pages/core/prefetch/prefetch_service_test_taco.cc
(right):

https://codereview.chromium.org/2920083002/diff/200001/components/offline_pag...
components/offline_pages/core/prefetch/prefetch_service_test_taco.cc:25: //
TODO(carlosk): initialize store_command_factory_ once a test implementation
On 2017/06/15 22:46:49, dewittj wrote:
> Isn't there a test one now?

Done.

[Dmitry Titov, 2017-06-16 01:30:46]

dimich@chromium.org changed reviewers:
+ dimich@chromium.org

[Dmitry Titov, 2017-06-16 01:30:48]

I'm not sure the method of handling zombies implemented here is robust. I know
it's what is in the design :) so sorry for randomization but looking at it makes
me think.

We don't really have any guarantees on how the 'batches of urls' coming from
Zine will be structured. For example, if we get 5 URLs, and then another 5 URLs,
and then the sequence repeats, we would be adding a lot of duplicates using the
login in this CL...

Would it be more reliable to keep 'old URLs' for longer and remove them by
expiration mechanism instead?

https://codereview.chromium.org/2920083002/diff/240001/components/offline_pag...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2920083002/diff/240001/components/offline_pag...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:38: for (const
PrefetchURL& prefetch_url : prefetch_urls_)
This doesn't feel right. Sprinkling the logic in Tasks, and then some of that
perhaps in Commands makes it hard to reason what should be done in Tasks and
what should be done in Commands.

Also, it prevents a single transaction to bracket the entire operation, which
makes it harder to re-play a command if the underlying storage has a problem.

In this case, it may be a single Command that does both operations (even if
logically it does sound as 2 things, it still can be a single Command).
Alternatively, there can be 2 Tasks with 1 Command each - in case if it's ok to
split these operations on 2 separate transactions, meaning that if the first
completes and second doesn't, the store remains in a consistent shape.

In general, Tasks could correspond 1-1 to Commands and to transaction
boundaries. In that kind of world we could re-run a command in SQL-based
Executor if we need to.

[commit-bot: I haz the power, 2017-06-16 01:51:23]

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

[commit-bot: I haz the power, 2017-06-16 01:51:24]

Dry run: This issue passed the CQ dry run.

[carlosk, 2017-06-16 16:50:34]

On 2017/06/16 01:30:48, Dmitry Titov wrote:
> I'm not sure the method of handling zombies implemented here is robust. I know
> it's what is in the design :) so sorry for randomization but looking at it
makes
> me think.
> 
> We don't really have any guarantees on how the 'batches of urls' coming from
> Zine will be structured. For example, if we get 5 URLs, and then another 5
URLs,
> and then the sequence repeats, we would be adding a lot of duplicates using
the
> login in this CL...
> 
> Would it be more reliable to keep 'old URLs' for longer and remove them by
> expiration mechanism instead?

Yes it makes sense to make zombies cleanup more robust as I was already seeing
some problems with this design. Maybe we could tie it to the lifetime of the
Offline Page Entries created from prefetched articles?

But I think that should be addressed in another CL.

https://codereview.chromium.org/2920083002/diff/240001/components/offline_pag...
File components/offline_pages/core/prefetch/add_unique_urls_task.cc (right):

https://codereview.chromium.org/2920083002/diff/240001/components/offline_pag...
components/offline_pages/core/prefetch/add_unique_urls_task.cc:38: for (const
PrefetchURL& prefetch_url : prefetch_urls_)
On 2017/06/16 01:30:47, Dmitry Titov wrote:
> This doesn't feel right. Sprinkling the logic in Tasks, and then some of that
> perhaps in Commands makes it hard to reason what should be done in Tasks and
> what should be done in Commands.

I agree the borderline is not clear but this seems to be an effect of our choice
of having a store interface with specialized calls instead of just CRUD.
Allowing elaborate operations makes it more difficult to draw that line.

The separation I see between Commands and Tasks is interaction with other
systems, what only Tasks should be able to do.

> Also, it prevents a single transaction to bracket the entire operation, which
> makes it harder to re-play a command if the underlying storage has a problem.
> 
> In this case, it may be a single Command that does both operations (even if
> logically it does sound as 2 things, it still can be a single Command).

Having these operations split in two commands is a choice but I also considered
them being a single command. I have no strong preference for one or the other.

But when considering your initial comment it seems zombies cleanup would end up
being a separate thing anyway.

> Alternatively, there can be 2 Tasks with 1 Command each - in case if it's ok
to
> split these operations on 2 separate transactions, meaning that if the first
> completes and second doesn't, the store remains in a consistent shape.
>
> In general, Tasks could correspond 1-1 to Commands and to transaction
> boundaries. In that kind of world we could re-run a command in SQL-based
> Executor if we need to.

This is where I disagree. This might be applicable for this (simple) pipeline
step but will not be for others where the result coming from the store affects
the input to another system (i.e. downloads) that might then cause an effect
back at the store. In the current design this step would have to be split in
multiple store commands but would be encompassed by a single task.

[carlosk, 2017-06-16 21:14:08]

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

[commit-bot: I haz the power, 2017-06-16 21:14:21]

Dry run: CQ is trying da patch.

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

[carlosk, 2017-06-16 21:29:54]

I merged Commands into tasks as discussed. PTAL.

[Dmitry Titov, 2017-06-16 22:47:06]

lgtm!

[dewittj, 2017-06-16 22:52:19]

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

[commit-bot: I haz the power, 2017-06-16 22:52:51]

Dry run: CQ is trying da patch.

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

[dewittj, 2017-06-17 00:06:14]

still lgtm

[commit-bot: I haz the power, 2017-06-17 00:29:54]

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

[commit-bot: I haz the power, 2017-06-17 00:29:55]

Dry run: This issue passed the CQ dry run.

[carlosk, 2017-06-17 05:15:47]

The CQ bit was checked by carlosk@chromium.org

[commit-bot: I haz the power, 2017-06-17 05:15:58]

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-17 09:54:50]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1497676547534730,
"parent_rev": "4d1dbe84650e7af780c1f701e34b7e1dcd8786d9", "commit_rev":
"12e48fc0602a0703d598c51577dbe918a3295914"}

[commit-bot: I haz the power, 2017-06-17 09:55:05]

Description was changed from

==========
Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

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

to

==========
Prefetching: Introduce store commands abstractions to be used by tasks.

Updates AddUniqueUrlsTask (still empty of actual business logic) to use store
commands as an abstraction layer for the execution of operations on the
persistent prefetching store. Only concrete test version of commands are
provided; actual SQL backer implementations will come later.

This also adds all the business logic needed for that task to work as the 1st
step of the prefetching pipeline but for NWake scheduling. The latter will
require a new abstraction to be created and will be left for a follow up patch.

BUG=701939

Review-Url: https://codereview.chromium.org/2920083002
Cr-Commit-Position: refs/heads/master@{#480292}
Committed:
https://chromium.googlesource.com/chromium/src/+/12e48fc0602a0703d598c51577db...
==========

[commit-bot: I haz the power, 2017-06-17 09:55:06]

Committed patchset #15 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/12e48fc0602a0703d598c51577db...