[Offline pages] OPMStoreSQL implementation of Add disabling updates

[Offline pages] OPMStoreSQL implementation of Add disabling updates

BUG=645522
Committed: https://crrev.com/0e80e6778c44ca1395fda39508a4f4fe0624490a
Cr-Commit-Position: refs/heads/master@{#418898}

[fgorski, 2016-09-15 03:25:58]

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

[commit-bot: I haz the power, 2016-09-15 03:26:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fgorski, 2016-09-15 03:27:08]

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

[fgorski, 2016-09-15 03:27:09]

PTAL (this is the simpler of the 2 patches ;) )

[commit-bot: I haz the power, 2016-09-15 04:11:27]

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

[commit-bot: I haz the power, 2016-09-15 04:11:28]

Dry run: This issue passed the CQ dry run.

[dewittj, 2016-09-15 14:58:27]

lg, a few comments

https://codereview.chromium.org/2331343007/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_metadata_store_sql.cc (right):

https://codereview.chromium.org/2331343007/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_metadata_store_sql.cc:190: " (offline_id,
online_url, client_namespace, client_id, file_path, "
Can we parametrize this  so that there's no chance that InsertOrReplace will get
out of sync with Insert?

https://codereview.chromium.org/2331343007/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_metadata_store_sql.cc:208: return
statement.Run() && db->GetLastChangeCount() > 0;
This function should return ItemActionStatus, so that statement.Run() == false
can be separated from LastChangeCount == 0.

[fgorski, 2016-09-15 16:52:49]

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

[commit-bot: I haz the power, 2016-09-15 16:53:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fgorski, 2016-09-15 16:53:47]

addressed.

https://codereview.chromium.org/2331343007/diff/1/components/offline_pages/of...
File components/offline_pages/offline_page_metadata_store_sql.cc (right):

https://codereview.chromium.org/2331343007/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_metadata_store_sql.cc:190: " (offline_id,
online_url, client_namespace, client_id, file_path, "
On 2016/09/15 14:58:26, dewittj wrote:
> Can we parametrize this  so that there's no chance that InsertOrReplace will
get
> out of sync with Insert?

No, and you will see why in the patch that follows.

https://codereview.chromium.org/2331343007/diff/1/components/offline_pages/of...
components/offline_pages/offline_page_metadata_store_sql.cc:208: return
statement.Run() && db->GetLastChangeCount() > 0;
On 2016/09/15 14:58:26, dewittj wrote:
> This function should return ItemActionStatus, so that statement.Run() == false
> can be separated from LastChangeCount == 0.

Done.

[Dmitry Titov, 2016-09-15 17:07:17]

lgtm

[dewittj, 2016-09-15 17:09:15]

lgtm

[fgorski, 2016-09-15 17:10:58]

The CQ bit was unchecked by fgorski@chromium.org

[fgorski, 2016-09-15 17:10:59]

The CQ bit was checked by fgorski@chromium.org

[commit-bot: I haz the power, 2016-09-15 17:11:16]

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, 2016-09-15 17:36:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-15 17:38:41]

Description was changed from

==========
[Offline pages] OPMStoreSQL implementation of Add disabling updates

BUG=645522
==========

to

==========
[Offline pages] OPMStoreSQL implementation of Add disabling updates

BUG=645522

Committed: https://crrev.com/0e80e6778c44ca1395fda39508a4f4fe0624490a
Cr-Commit-Position: refs/heads/master@{#418898}
==========

[commit-bot: I haz the power, 2016-09-15 17:38:42]

Patchset 2 (id:??) landed as
https://crrev.com/0e80e6778c44ca1395fda39508a4f4fe0624490a
Cr-Commit-Position: refs/heads/master@{#418898}