Split ExtensionSystem interface from ExtensionSystemImpl implementation, part 1.

Split ExtensionSystem interface from ExtensionSystemImpl implementation, part 1.

This moves ExtensionSystem to extensions/browser.
It also moves setting the channel for Features to ChromeExtensionsBrowserClient.
It also removes the duplicate ExtensionSystem::GetForBrowserContext.

BUG=337707
TBR=sky@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=247788

[Yoyo Zhou, 2014-01-28 22:25:50]

jamescook: Please review.
sky: TBR for uninteresting refactoring changes.

[James Cook, 2014-01-28 23:40:54]

Nice improvement.

If you have to do a similarly large patch in the future it would be nice to do
the mechanical file move separately in a patch that could be rubber-stamped.  It
took me a long time to just flip through all the files.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/chrome...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/extensions/input_method_api.cc:87:
ExtensionSystem::Get(context_)->event_router()->
wrap line

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/extensions/input_method_api.cc:96:
ExtensionSystem::Get(context_)->event_router()->
ditto. Maybe git cl format would be useful.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
File chrome/browser/extensions/api/identity/web_auth_flow.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
chrome/browser/extensions/api/identity/web_auth_flow.cc:30: #include
"extensions/browser/extension_system.h"
Heh. Funny. The file move tool maintained the error.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
File chrome/browser/extensions/api/storage/settings_apitest.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
chrome/browser/extensions/api/storage/settings_apitest.cc:462:
ExtensionSystemFactory::GetForBrowserContext(profile);
Maybe just ExtensionSystem::Get(profile)?

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
File chrome/browser/extensions/extension_system_impl.h (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
chrome/browser/extensions/extension_system_impl.h:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
2014?

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/sync/p...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/sync/p...
chrome/browser/sync/profile_sync_components_factory_impl.cc:127:
extensions::ExtensionSystemFactory::GetForBrowserContext(profile)),
ExtensionSystem::Get()?

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/ui/omn...
File chrome/browser/ui/omnibox/location_bar.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/ui/omn...
chrome/browser/ui/omnibox/location_bar.cc:24: extensions::ExtensionSystem::Get(
wrap?

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ev...
File extensions/browser/event_router.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ev...
extensions/browser/event_router.cc:85: ExtensionSystem::Get(context);
Can this be rewrapped?

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ev...
extensions/browser/event_router.cc:577:
ExtensionSystem::Get(context)->process_manager();
ditto?

There might be a way to run clang-format just on the lines affected by the
patch. git cl format?

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
File extensions/browser/extension_system.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.cc:6: #include
"chrome/browser/extensions/extension_system_factory.h"
optional nit: blank line above?

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
File extensions/browser/extension_system.h (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:11: #include "base/memory/scoped_ptr.h"
do you need this header?

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:14: #include
"extensions/common/one_shot_event.h"
I don't think you need this header, probably can forward declare it

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:17: class Profile;
don't think you need this

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:48: // This interface supports using
TestExtensionSystem for TestingProfiles
comment nit: Blank line above. Also, it would be nice if this comment described
what ExtensionSystem is.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:57: static ExtensionSystem*
Get(content::BrowserContext* context);
Hooray, one Get() method to rule them all!

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:60: virtual void Shutdown() OVERRIDE {}
optional nit: Maybe just not override it, since the default implementation is
{}?

[Yoyo Zhou, 2014-01-29 00:42:53]

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/chrome...
File chrome/browser/chromeos/extensions/input_method_api.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/chrome...
chrome/browser/chromeos/extensions/input_method_api.cc:87:
ExtensionSystem::Get(context_)->event_router()->
On 2014/01/28 23:40:57, James Cook wrote:
> wrap line

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
File chrome/browser/extensions/api/identity/web_auth_flow.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
chrome/browser/extensions/api/identity/web_auth_flow.cc:30: #include
"extensions/browser/extension_system.h"
On 2014/01/28 23:40:57, James Cook wrote:
> Heh. Funny. The file move tool maintained the error.

Fixed.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
File chrome/browser/extensions/api/storage/settings_apitest.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
chrome/browser/extensions/api/storage/settings_apitest.cc:462:
ExtensionSystemFactory::GetForBrowserContext(profile);
On 2014/01/28 23:40:57, James Cook wrote:
> Maybe just ExtensionSystem::Get(profile)?

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
File chrome/browser/extensions/extension_system_impl.h (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/extens...
chrome/browser/extensions/extension_system_impl.h:1: // Copyright 2013 The
Chromium Authors. All rights reserved.
On 2014/01/28 23:40:57, James Cook wrote:
> 2014?

Ok, fine. =)

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/sync/p...
File chrome/browser/sync/profile_sync_components_factory_impl.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/sync/p...
chrome/browser/sync/profile_sync_components_factory_impl.cc:127:
extensions::ExtensionSystemFactory::GetForBrowserContext(profile)),
On 2014/01/28 23:40:57, James Cook wrote:
> ExtensionSystem::Get()?

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/ui/omn...
File chrome/browser/ui/omnibox/location_bar.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/chrome/browser/ui/omn...
chrome/browser/ui/omnibox/location_bar.cc:24: extensions::ExtensionSystem::Get(
On 2014/01/28 23:40:57, James Cook wrote:
> wrap?

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ev...
File extensions/browser/event_router.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ev...
extensions/browser/event_router.cc:85: ExtensionSystem::Get(context);
On 2014/01/28 23:40:57, James Cook wrote:
> Can this be rewrapped?

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
File extensions/browser/extension_system.cc (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.cc:6: #include
"chrome/browser/extensions/extension_system_factory.h"
On 2014/01/28 23:40:57, James Cook wrote:
> optional nit: blank line above?

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
File extensions/browser/extension_system.h (right):

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:11: #include "base/memory/scoped_ptr.h"
On 2014/01/28 23:40:57, James Cook wrote:
> do you need this header?

Nope.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:14: #include
"extensions/common/one_shot_event.h"
On 2014/01/28 23:40:57, James Cook wrote:
> I don't think you need this header, probably can forward declare it

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:17: class Profile;
On 2014/01/28 23:40:57, James Cook wrote:
> don't think you need this

Done.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:48: // This interface supports using
TestExtensionSystem for TestingProfiles
On 2014/01/28 23:40:57, James Cook wrote:
> comment nit: Blank line above. Also, it would be nice if this comment
described
> what ExtensionSystem is.

Added a comment.

https://chromiumcodereview.appspot.com/147923005/diff/1/extensions/browser/ex...
extensions/browser/extension_system.h:60: virtual void Shutdown() OVERRIDE {}
On 2014/01/28 23:40:57, James Cook wrote:
> optional nit: Maybe just not override it, since the default implementation is
> {}?

Done.

[James Cook, 2014-01-29 18:24:43]

LGTM

[commit-bot: I haz the power, 2014-01-29 19:36:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoz@chromium.org/147923005/70001

[commit-bot: I haz the power, 2014-01-30 00:41:30]

Change committed as 247788