Attaches OfflineContentAggregator to Profile

Attaches OfflineContentAggregator to Profile

This CL hooks the OfflineContentAggregator to a BrowserContext and adds
a Java bridge to get access to the Java component as well.

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

[David Trainor- moved to gerrit, 2017-03-06 21:47:22]

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

[David Trainor- moved to gerrit, 2017-03-06 21:47:23]

hooking up the OCP to the Profile and adding the Java bridge.  ptal thanks!

[Dmitry Titov, 2017-03-08 00:46:01]

https://codereview.chromium.org/2729983002/diff/1/components/offline_items_co...
File
components/offline_items_collection/content/offline_content_aggregator_factory.cc
(right):

https://codereview.chromium.org/2729983002/diff/1/components/offline_items_co...
components/offline_items_collection/content/offline_content_aggregator_factory.cc:23:
DCHECK(!context->IsOffTheRecord());
It is unclear if you want the service to be created for Incognito or not. The
override of GetBrowserContextToUse seems to skip the default implementation
which returns nullptr for Incognito profile. So it looks one part of the code
wants to specifically enable Incognito profile while another is DCHECK'ing
against it.

[fgorski, 2017-03-08 17:42:28]

looks ok mod comment from Dmitry.

[David Trainor- moved to gerrit, 2017-03-09 18:44:40]

https://codereview.chromium.org/2729983002/diff/1/components/offline_items_co...
File
components/offline_items_collection/content/offline_content_aggregator_factory.cc
(right):

https://codereview.chromium.org/2729983002/diff/1/components/offline_items_co...
components/offline_items_collection/content/offline_content_aggregator_factory.cc:23:
DCHECK(!context->IsOffTheRecord());
On 2017/03/08 00:46:00, Dmitry Titov wrote:
> It is unclear if you want the service to be created for Incognito or not. The
> override of GetBrowserContextToUse seems to skip the default implementation
> which returns nullptr for Incognito profile. So it looks one part of the code
> wants to specifically enable Incognito profile while another is DCHECK'ing
> against it.

Ah nice catch sorry!  I was messing around with the ordering and mixed it up. 
Thanks :).  Looks like I'll have to move this class out of the component and
into chrome/.  I think the only way to grab the original BrowserContext is by
using GetBrowserContextRedirectedInIncognito().

[David Trainor- moved to gerrit, 2017-03-16 05:00:11]

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

[commit-bot: I haz the power, 2017-03-16 05:00:42]

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-16 05:04:26]

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

[commit-bot: I haz the power, 2017-03-16 05:04:27]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[David Trainor- moved to gerrit, 2017-03-16 15:10:35]

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

[commit-bot: I haz the power, 2017-03-16 15:10:57]

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-16 17:08:11]

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

[commit-bot: I haz the power, 2017-03-16 17:08:12]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[David Trainor- moved to gerrit, 2017-03-16 19:18:46]

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

[commit-bot: I haz the power, 2017-03-16 19:19:36]

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-16 20:24:49]

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

[commit-bot: I haz the power, 2017-03-16 20:24:51]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-03-16 21:48:39]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-16 21:48:45]

The CQ bit was unchecked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-16 21:50:30]

Should be addressed.  ptal thanks!

[qinmin, 2017-03-17 20:50:56]

lgtm % nit

https://codereview.chromium.org/2729983002/diff/60001/chrome/browser/android/...
File
chrome/browser/android/download/items/offline_content_aggregator_factory_android.cc
(right):

https://codereview.chromium.org/2729983002/diff/60001/chrome/browser/android/...
chrome/browser/android/download/items/offline_content_aggregator_factory_android.cc:18:
namespace content {
nit: still need this?

[Dmitry Titov, 2017-03-17 21:43:06]

lgtm

[fgorski, 2017-03-17 21:53:42]

lgtm

[David Trainor- moved to gerrit, 2017-03-20 05:47:19]

https://codereview.chromium.org/2729983002/diff/60001/chrome/browser/android/...
File
chrome/browser/android/download/items/offline_content_aggregator_factory_android.cc
(right):

https://codereview.chromium.org/2729983002/diff/60001/chrome/browser/android/...
chrome/browser/android/download/items/offline_content_aggregator_factory_android.cc:18:
namespace content {
On 2017/03/17 20:50:56, qinmin wrote:
> nit: still need this?

Done.

[David Trainor- moved to gerrit, 2017-03-20 05:47:37]

The CQ bit was checked by dtrainor@chromium.org

[David Trainor- moved to gerrit, 2017-03-20 05:47:37]

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

[commit-bot: I haz the power, 2017-03-20 05:47:55]

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 05:51:24]

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

[commit-bot: I haz the power, 2017-03-20 05:51:25]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

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

The CQ bit was checked by dtrainor@chromium.org

[commit-bot: I haz the power, 2017-03-20 06:47:06]

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 06:53:36]

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

[commit-bot: I haz the power, 2017-03-20 06:53:37]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

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

dtrainor@chromium.org changed reviewers:
+ erg@chromium.org

[David Trainor- moved to gerrit, 2017-03-20 15:29:44]

erg@ do you mind doing a DEPS review for components/keyed_service? Thanks!

[Elliot Glaysher, 2017-03-20 17:23:54]

lgtm

I continue to think the separation of keyed services and keyed service factories
into different toplevel components is a weird pattern, but that ship sailed a
long time ago. :(

[David Trainor- moved to gerrit, 2017-03-20 20:05:57]

On 2017/03/20 17:23:54, Elliot Glaysher wrote:
> lgtm
> 
> I continue to think the separation of keyed services and keyed service
factories
> into different toplevel components is a weird pattern, but that ship sailed a
> long time ago. :(

I think the main issue was I wasn't sure how to have both off the record and on
the record contexts hitting the same OfflineContentAggregator.  If there's a way
to do that in the component I'd be happy to keep the factory there :).

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

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

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

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 20:13:26]

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

[commit-bot: I haz the power, 2017-03-20 20:13:27]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2017-03-20 21:01:34]

The CQ bit was checked by dtrainor@chromium.org

[commit-bot: I haz the power, 2017-03-20 21:03:26]

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 21:14:26]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490043694969530,
"parent_rev": "3b149b2d2e71280dc70df934427df29e9ee25169", "commit_rev":
"e43c093fdaa7ce7eb41a084a15ff5fa12f08537d"}

[commit-bot: I haz the power, 2017-03-20 21:15:13]

Description was changed from

==========
Attaches OfflineContentAggregator to Profile

This CL hooks the OfflineContentAggregator to a BrowserContext and adds
a Java bridge to get access to the Java component as well.

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

to

==========
Attaches OfflineContentAggregator to Profile

This CL hooks the OfflineContentAggregator to a BrowserContext and adds
a Java bridge to get access to the Java component as well.

BUG=691805

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

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

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/e43c093fdaa7ce7eb41a084a15ff...