Support providers with two namespaces

Support providers with two namespaces

OfflinePages requires a single provider to be able to handle exposing
items with two namespaces.  This CL adds support for that in the
OfflineContentAggregator.  A provider can be registered for more than
one namespace at once.  Also adds a test to validate that the dual
namespaces works as expected and documents the few odd behavior points
that aren't currently supported.

BUG=691805
Review-Url: https://codereview.chromium.org/2757773002
Cr-Commit-Position: refs/heads/master@{#458879}
Committed: https://chromium.googlesource.com/chromium/src/+/da65ddd7e896738a9ccbeacf277105394d429291

[David Trainor- moved to gerrit, 2017-03-16 21:55:00]

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

[David Trainor- moved to gerrit, 2017-03-16 21:55:01]

ptal thanks!

[fgorski, 2017-03-17 04:15:16]

lgtm % comments.

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
File components/offline_items_collection/core/offline_content_aggregator.cc
(right):

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
components/offline_items_collection/core/offline_content_aggregator.cc:44: //
with any other namespaces.
nit: would singular namespace work here?

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
File components/offline_items_collection/core/offline_content_aggregator.h
(right):

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
components/offline_items_collection/core/offline_content_aggregator.h:69: //    
 any items for a namespace that it didn't register.
nit: close )

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
File components/offline_items_collection/core/offline_item.cc (right):

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
components/offline_items_collection/core/offline_item.cc:65: return id <
offline_item.id;
this is a little dangerous as (!(a < b) && !(b < a)) should imply equality,
which will be inconsistent with implementation of operator== . At least
documenting that would be useful.

[Dmitry Titov, 2017-03-18 01:24:04]

lgtm % Filip's comments

[David Trainor- moved to gerrit, 2017-03-20 06:43:43]

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

[commit-bot: I haz the power, 2017-03-20 06:44: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-03-20 07:27:59]

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

[commit-bot: I haz the power, 2017-03-20 07:28:03]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-03-22 00:12:45]

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
File components/offline_items_collection/core/offline_content_aggregator.cc
(right):

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
components/offline_items_collection/core/offline_content_aggregator.cc:44: //
with any other namespaces.
On 2017/03/17 04:15:15, fgorski wrote:
> nit: would singular namespace work here?

Done.

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
File components/offline_items_collection/core/offline_content_aggregator.h
(right):

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
components/offline_items_collection/core/offline_content_aggregator.h:69: //    
 any items for a namespace that it didn't register.
On 2017/03/17 04:15:15, fgorski wrote:
> nit: close )

Done.

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
File components/offline_items_collection/core/offline_item.cc (right):

https://codereview.chromium.org/2757773002/diff/1/components/offline_items_co...
components/offline_items_collection/core/offline_item.cc:65: return id <
offline_item.id;
On 2017/03/17 04:15:16, fgorski wrote:
> this is a little dangerous as (!(a < b) && !(b < a)) should imply equality,
> which will be inconsistent with implementation of operator== . At least
> documenting that would be useful.

Yeah that's a good point.  I didn't like this either.  I only needed it for a
unit test so I just wrote a custom comparator there for std::map.

[David Trainor- moved to gerrit, 2017-03-22 00:13:32]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-22 00:13:32]

The patchset sent to the CQ was uploaded after l-g-t-m from dimich@chromium.org,
fgorski@chromium.org
Link to the patchset: https://codereview.chromium.org/2757773002/#ps20001
(title: "Address comments")

[commit-bot: I haz the power, 2017-03-22 00:14:10]

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-03-22 02:15:26]

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

[commit-bot: I haz the power, 2017-03-22 02:15:27]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[David Trainor- moved to gerrit, 2017-03-22 21:08:35]

The CQ bit was checked by dtrainor@chromium.org

[commit-bot: I haz the power, 2017-03-22 21:09: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, 2017-03-22 21:18:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490216915405420,
"parent_rev": "97a462d45974e7a4eb0e033730abd8a78aa55e64", "commit_rev":
"da65ddd7e896738a9ccbeacf277105394d429291"}

[commit-bot: I haz the power, 2017-03-22 21:19:14]

Description was changed from

==========
Support providers with two namespaces

OfflinePages requires a single provider to be able to handle exposing
items with two namespaces.  This CL adds support for that in the
OfflineContentAggregator.  A provider can be registered for more than
one namespace at once.  Also adds a test to validate that the dual
namespaces works as expected and documents the few odd behavior points
that aren't currently supported.

BUG=691805
==========

to

==========
Support providers with two namespaces

OfflinePages requires a single provider to be able to handle exposing
items with two namespaces.  This CL adds support for that in the
OfflineContentAggregator.  A provider can be registered for more than
one namespace at once.  Also adds a test to validate that the dual
namespaces works as expected and documents the few odd behavior points
that aren't currently supported.

BUG=691805

Review-Url: https://codereview.chromium.org/2757773002
Cr-Commit-Position: refs/heads/master@{#458879}
Committed:
https://chromium.googlesource.com/chromium/src/+/da65ddd7e896738a9ccbeacf2771...
==========

[commit-bot: I haz the power, 2017-03-22 21:19:15]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/da65ddd7e896738a9ccbeacf2771...