[Offline Pages] Fix cases where returning NOT_FOUND when deleting.

[Offline Pages] Fix cases where returning NOT_FOUND when deleting.

We were returning NOT_FOUND when trying to delete an empty list of
offline ids instead of SUCCESS. NOT_FOUND should only be used only when
some ids that were passed in cannot be found in the database.

BUG=608057
Committed: https://crrev.com/2c0a99cc4e0b602137448367b4b070875e6e0d67
Cr-Commit-Position: refs/heads/master@{#401353}

[romax, 2016-06-20 23:22:44]

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

[romax, 2016-06-20 23:22:44]

PTAL. I'm not sure if the DeletePagesByURLPredicate should return NOT_FOUND when
there's no page found. I'm leaning towards returning NOT_FOUND when there's no
page found for the condition passed in (or some of the offline ids are not in
metadata store).

[fgorski, 2016-06-21 18:02:24]

Hey, per your comment to the patch and our discussion. Let's get rid of
NOT_FOUND.

We need to keep the enum value, but let's deprecate it (add a comment to it that
it should not be used.

You will also have to update the java test.

https://codereview.chromium.org/2086733002/diff/20001/components/offline_page...
File components/offline_pages/offline_page_model_impl.cc (right):

https://codereview.chromium.org/2086733002/diff/20001/components/offline_page...
components/offline_pages/offline_page_model_impl.cc:358: // Shortcut here, if
none of the offline id is found and no other valid
Too long of the comment.

If there are no pages to delete, return early.

https://codereview.chromium.org/2086733002/diff/20001/components/offline_page...
components/offline_pages/offline_page_model_impl.cc:851: bool page_not_found =
false;
I don't think we need this logic.
If you identified a page for deletion earlier, you consider it a success when
the page is gone.

[romax, 2016-06-21 19:41:37]

PTAL

https://codereview.chromium.org/2086733002/diff/20001/components/offline_page...
File components/offline_pages/offline_page_model_impl.cc (right):

https://codereview.chromium.org/2086733002/diff/20001/components/offline_page...
components/offline_pages/offline_page_model_impl.cc:358: // Shortcut here, if
none of the offline id is found and no other valid
On 2016/06/21 18:02:23, fgorski wrote:
> Too long of the comment.
> 
> If there are no pages to delete, return early.

Done.

https://codereview.chromium.org/2086733002/diff/20001/components/offline_page...
components/offline_pages/offline_page_model_impl.cc:851: bool page_not_found =
false;
On 2016/06/21 18:02:23, fgorski wrote:
> I don't think we need this logic.
> If you identified a page for deletion earlier, you consider it a success when
> the page is gone.

Done.

[fgorski, 2016-06-21 19:52:24]

lgtm with nit

https://codereview.chromium.org/2086733002/diff/40001/components/offline_page...
File components/offline_pages/offline_page_types.h (right):

https://codereview.chromium.org/2086733002/diff/40001/components/offline_page...
components/offline_pages/offline_page_types.h:51: NOT_FOUND,  // Deprecated.
Should not be used anymore.
nit: please align to 2 spaces after rightmost coma (the one in DEVICE_FAILURE

[romax, 2016-06-22 18:10:44]

The CQ bit was checked by romax@chromium.org

[romax, 2016-06-22 18:10:44]

The patchset sent to the CQ was uploaded after l-g-t-m from fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2086733002/#ps60001
(title: "Fix comments.")

[commit-bot: I haz the power, 2016-06-22 18:11:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2086733002/60001

[commit-bot: I haz the power, 2016-06-22 18:19:26]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-22 18:22:39]

Description was changed from

==========
[Offline Pages] Fix cases where returning NOT_FOUND when deleting.

We were returning NOT_FOUND when trying to delete an empty list of
offline ids instead of SUCCESS. NOT_FOUND should only be used only when
some ids that were passed in cannot be found in the database.

BUG=608057
==========

to

==========
[Offline Pages] Fix cases where returning NOT_FOUND when deleting.

We were returning NOT_FOUND when trying to delete an empty list of
offline ids instead of SUCCESS. NOT_FOUND should only be used only when
some ids that were passed in cannot be found in the database.

BUG=608057

Committed: https://crrev.com/2c0a99cc4e0b602137448367b4b070875e6e0d67
Cr-Commit-Position: refs/heads/master@{#401353}
==========

[commit-bot: I haz the power, 2016-06-22 18:22:40]

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