Extract ErrorConsole from ExtensionSystem.

Extract ErrorConsole from ExtensionSystem.

This patch removes error_console accessor from ExtensionSystem.
It can be its own browser context keyed service. And it can be
built by its new factory.

BUG=

Committed: https://crrev.com/60673b43ede9dc07633683d9176d113c647aa8da
Cr-Commit-Position: refs/heads/master@{#329480}

[juncai, 2015-05-12 17:48:41]

juncai@chromium.org changed reviewers:
+ reillyg@chromium.org

[juncai, 2015-05-12 17:48:41]

Please take a look at this patch.

[Reilly Grant (use Gerrit), 2015-05-12 17:58:53]

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/c...
File chrome/browser/extensions/chrome_extension_web_contents_observer.cc
(right):

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/c...
chrome/browser/extensions/chrome_extension_web_contents_observer.cc:64:
ErrorConsoleFactory::GetForBrowserContext(browser_context())->ReportError(
Use ErrorConsole::Get instead of including error_console_factory.h.

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/error_console/error_console_factory.cc (right):

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/error_console/error_console_factory.cc:1: // Copyright
2014 The Chromium Authors. All rights reserved.
2014 -> 2015

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/error_console/error_console_factory.h (right):

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/error_console/error_console_factory.h:1: // Copyright
2014 The Chromium Authors. All rights reserved.
2014 -> 2015

[juncai, 2015-05-12 19:01:01]

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/c...
File chrome/browser/extensions/chrome_extension_web_contents_observer.cc
(right):

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/c...
chrome/browser/extensions/chrome_extension_web_contents_observer.cc:64:
ErrorConsoleFactory::GetForBrowserContext(browser_context())->ReportError(
On 2015/05/12 17:58:52, reillyg wrote:
> Use ErrorConsole::Get instead of including error_console_factory.h.

Done.

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/error_console/error_console_factory.cc (right):

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/error_console/error_console_factory.cc:1: // Copyright
2014 The Chromium Authors. All rights reserved.
On 2015/05/12 17:58:52, reillyg wrote:
> 2014 -> 2015

Done.

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/error_console/error_console_factory.h (right):

https://codereview.chromium.org/1139643005/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/error_console/error_console_factory.h:1: // Copyright
2014 The Chromium Authors. All rights reserved.
On 2015/05/12 17:58:52, reillyg wrote:
> 2014 -> 2015

Done.

[Reilly Grant (use Gerrit), 2015-05-12 19:14:38]

lgtm

[juncai, 2015-05-12 19:15:21]

The CQ bit was checked by juncai@chromium.org

[commit-bot: I haz the power, 2015-05-12 19:16:46]

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

[commit-bot: I haz the power, 2015-05-12 20:47:04]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-12 20:48:59]

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

[Devlin, 2015-06-17 17:30:44]

On 2015/05/12 20:48:59, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/60673b43ede9dc07633683d9176d113c647aa8da
> Cr-Commit-Position: refs/heads/master@{#329480}

(Sorry to comment on a closed CL, but it looks like there was never a bug
associated with it.)

I'm not sure I see the benefit of this.  Making things keyed services *greatly*
complicates the lifetime associated with them, adds its own set of boilerplate
(the factory), and doesn't seem to have really gained us anything.  What was the
motivation for this and related patches?

[Reilly Grant (use Gerrit), 2015-06-17 17:51:27]

On 2015/06/17 17:30:44, Devlin wrote:
> On 2015/05/12 20:48:59, commit-bot: I haz the power wrote:
> > Patchset 2 (id:??) landed as
> > https://crrev.com/60673b43ede9dc07633683d9176d113c647aa8da
> > Cr-Commit-Position: refs/heads/master@{#329480}
> 
> (Sorry to comment on a closed CL, but it looks like there was never a bug
> associated with it.)
> 
> I'm not sure I see the benefit of this.  Making things keyed services
*greatly*
> complicates the lifetime associated with them, adds its own set of boilerplate
> (the factory), and doesn't seem to have really gained us anything.  What was
the
> motivation for this and related patches?

By separating the large ExtensionSystem keyed service into smaller pieces we
make each a little more complex but in exchange we can reason about them
independently. This simplifies consumers of this code because they only need to
depend on the service that they need instead of a bundle of services that is
harder to initialize because it has deeper dependencies. The benefit is seen
most strongly in tests. Note that, aside from the addition of the factory class,
which I agree is sadly a bit of boilerplate, this patch is a net removal of
code.

Last time I worked on this people seemed to like this idea so I thought it would
be good for juncai@ to work on to explore the extensions system and some core
Chrome design patterns. It seems that that opinion has shifted and we aren't
working on this type of patch anymore. I apologize for the churn if that's all
you see this as.