Use EventRouter::Get instead of ExtensionSystem::Get(browser_context)->event_router()

Use EventRouter::Get instead of ExtensionSystem::Get(browser_context)->event_router()
from c/b/e/api

and remove unwanted extensions:: prefix.

R=rdevlin.cronin@chromium.org
BUG=362672
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271590

[limasdf, 2014-05-16 10:21:21]

ptal. this is simple cleanup.

[Devlin, 2014-05-16 15:32:53]

Couple of small requests. :)

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/automation_internal/automation_util.cc
(right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/automation_internal/automation_util.cc:15:
#include "extensions/browser/extension_system.h"
I think you can safely remove extension_system.h now.

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/downloads/downloads_api.cc (right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/downloads/downloads_api.cc:284: const
extensions::Extension* extension = extensions::ExtensionSystem::Get(
This is totally unrelated to this patch... but since you're here :)

Can you make this:
const Extension* extension = ExtensionRegistry::Get(profile)
    ->GetExtensionById(by_ext->id(), ExtensionRegistry::EVERYTHING);

And then you can take out the extension_system.h and extension_service.h
includes above.

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/downloads/downloads_api.cc:1740: const
extensions::EventListenerInfo& details) {
Can you take out the extensions:: prefixes in this file, too?

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
(right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:300:
extensions::EventRouter::Get(current_browser()->profile())
Since this is in the extensions namespace, can you remove the extensions::
prefix? (Here and everywhere else in this file.)

[limasdf, 2014-05-16 16:25:03]

thank you for the review! ptal.

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/automation_internal/automation_util.cc
(right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/automation_internal/automation_util.cc:15:
#include "extensions/browser/extension_system.h"
On 2014/05/16 15:32:53, D Cronin wrote:
> I think you can safely remove extension_system.h now.

Done. I blame myself about forget this :/

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/downloads/downloads_api.cc (right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/downloads/downloads_api.cc:284: const
extensions::Extension* extension = extensions::ExtensionSystem::Get(
On 2014/05/16 15:32:53, D Cronin wrote:
> This is totally unrelated to this patch... but since you're here :)
> 
> Can you make this:
> const Extension* extension = ExtensionRegistry::Get(profile)
>     ->GetExtensionById(by_ext->id(), ExtensionRegistry::EVERYTHING);
> 
> And then you can take out the extension_system.h and extension_service.h
> includes above.

Done.

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/downloads/downloads_api.cc:1740: const
extensions::EventListenerInfo& details) {
On 2014/05/16 15:32:53, D Cronin wrote:
> Can you take out the extensions:: prefixes in this file, too?

Done.

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
(right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:300:
extensions::EventRouter::Get(current_browser()->profile())
On 2014/05/16 15:32:53, D Cronin wrote:
> Since this is in the extensions namespace, can you remove the extensions::
> prefix? (Here and everywhere else in this file.)

Done.

[Devlin, 2014-05-16 17:34:04]

A few more comments.  Also, please update the patch title to reflect that now
this is also removing deprecated notifications (unless you wanted that in a
separate CL). :)

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/automation_internal/automation_util.cc
(right):

https://codereview.chromium.org/280853004/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/automation_internal/automation_util.cc:15:
#include "extensions/browser/extension_system.h"
On 2014/05/16 16:25:03, sunglim wrote:
> On 2014/05/16 15:32:53, D Cronin wrote:
> > I think you can safely remove extension_system.h now.
> 
> Done. I blame myself about forget this :/

Haha don't be so hard on yourself. We all forget. :)

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/app_list/start_page_handler.cc (right):

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/app_list/start_page_handler.cc:97:
content::Source<Profile>(source).ptr());
This won't compile - the |source| variable is gone.
Instead:
DCHECK_EQ(Profile::FromWebUI(web_ui()),
          Profile::FromBrowserContext(browser_context));

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/app_list/start_page_handler.cc:108:
DCHECK_EQ(Profile::FromWebUI(web_ui()),
same as above.

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/app_list/start_page_handler.cc:143:
extension_service->GetExtensionById(
Please change this to be ExtensionRegistry::GetExtensionById().  The fewer times
we can call into ExtensionService for stuff like this (especially after
notifications) the better.

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/app_list/start_page_handler.h (right):

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/app_list/start_page_handler.h:40: //
extension::ExtensionRegistryObserver implementation.
micro nit: extensions::, not extension::.

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/command_handler.cc (right):

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/command_handler.cc:145: const ExtensionSet*
extensions = extensions::ExtensionSystem::Get(profile)->
Please change to ExtensionRegistry::enabled_extensions() (and then you can
remove the extension_service include).

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/extensions/command_handler.h (right):

https://codereview.chromium.org/280853004/diff/10001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/extensions/command_handler.h:47: //
extension::ExtensionRegistryObserver implementation.
nit: extensions::

[limasdf, 2014-05-17 13:32:21]

@cronin, ptal. I removed change which is not in commit description. 
I'll upload those thing in seperate CL.

and My account(limasdf@gmail.com) is registered in crbug.com.
if you need a help(I'm happy to do chores or mechanical change), you can CC me
:)

[Devlin, 2014-05-19 15:50:58]

lgtm with two small nits. :)

https://codereview.chromium.org/280853004/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
(right):

https://codereview.chromium.org/280853004/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:27:
#include "chrome/browser/extensions/extension_service.h"
nit: Know it's not yours, but please remove this if it's not needed (doesn't
look like it is).

https://codereview.chromium.org/280853004/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:46:
#include "content/test/net/url_request_slow_download_job.h"
nit: Please add extensions/browser/event_router.h

[limasdf, 2014-05-20 00:26:02]

Thanks for the review!

https://codereview.chromium.org/280853004/diff/30001/chrome/browser/extension...
File chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc
(right):

https://codereview.chromium.org/280853004/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:27:
#include "chrome/browser/extensions/extension_service.h"
On 2014/05/19 15:50:58, D Cronin wrote:
> nit: Know it's not yours, but please remove this if it's not needed (doesn't
> look like it is).

Done.

https://codereview.chromium.org/280853004/diff/30001/chrome/browser/extension...
chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc:46:
#include "content/test/net/url_request_slow_download_job.h"
On 2014/05/19 15:50:58, D Cronin wrote:
> nit: Please add extensions/browser/event_router.h

Done.

[limasdf, 2014-05-20 00:26:07]

The CQ bit was checked by limasdf@gmail.com

[commit-bot: I haz the power, 2014-05-20 00:27:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/limasdf@gmail.com/280853004/50001

[commit-bot: I haz the power, 2014-05-20 03:49:45]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-20 06:32:06]

Change committed as 271590