Moved ExternalExtensionLoaders/Providers into extensions namespace

Moved ExternalExtensionLoaders/Providers into extensions namespace

Moved ExternalExtensionLoader, ExternalExtensionProviderImpl,
ExternalExtensionProviderInterface, ExternalRegistryExtensionLoader,
ExternalPrefsExtensionLoader, and ExternalPolicyExtensionLoader into extensions
namespace. Renamed appropriately.

BUG=136873, 117261
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=146847

[Devlin, 2012-07-12 22:01:31]

Please review when time allows.
kalman: c/b/extensions
pastarmovj: c/b/policy

Thanks in advance.

[pastarmovj, 2012-07-12 22:23:18]

I think Joao knows this code better and although I don't see an issue with this
trivial change I will let him stamp this one.

[Joao da Silva, 2012-07-13 13:33:42]

lgtm after fixing the nits and a bot run.

IMO adding the extensions:: prefix to classes mentioned in comments decreases
their readability, and I'd leave them out.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/default_apps.h (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/default_apps.h:56: 
Nit: remove this extra newline

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/extension_service_unittest.cc (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/extension_service_unittest.cc:43: #include
"chrome/browser/extensions/external_pref_loader.h"
Nit: order

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_policy_loader_unittest.cc (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_policy_loader_unittest.cc:13: #include
"chrome/browser/extensions/external_policy_loader.h"
Nit: order

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_provider_impl.cc:20: #include
"chrome/browser/extensions/external_pref_loader.h"
Nit: order

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_provider_interface.h (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_provider_interface.h:55: const
ExternalProviderInterface* provider) = 0;
Nit: indent

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_registry_loader_win.h (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_registry_loader_win.h:1: // Copyright (c)
2011 The Chromium Authors. All rights reserved.
2012

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/chrome_browser_...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/chrome_browser_...
chrome/chrome_browser_extensions.gypi:416:
'browser/extensions/external_pref_loader.h',
Nit: order

[Devlin, 2012-07-13 16:31:03]

On 2012/07/13 13:33:42, Joao da Silva wrote:
> lgtm after fixing the nits and a bot run.
> 
> IMO adding the extensions:: prefix to classes mentioned in comments decreases
> their readability, and I'd leave them out.
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> File chrome/browser/extensions/default_apps.h (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> chrome/browser/extensions/default_apps.h:56: 
> Nit: remove this extra newline
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> File chrome/browser/extensions/extension_service_unittest.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> chrome/browser/extensions/extension_service_unittest.cc:43: #include
> "chrome/browser/extensions/external_pref_loader.h"
> Nit: order
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> File chrome/browser/extensions/external_policy_loader_unittest.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> chrome/browser/extensions/external_policy_loader_unittest.cc:13: #include
> "chrome/browser/extensions/external_policy_loader.h"
> Nit: order
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> File chrome/browser/extensions/external_provider_impl.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> chrome/browser/extensions/external_provider_impl.cc:20: #include
> "chrome/browser/extensions/external_pref_loader.h"
> Nit: order
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> File chrome/browser/extensions/external_provider_interface.h (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> chrome/browser/extensions/external_provider_interface.h:55: const
> ExternalProviderInterface* provider) = 0;
> Nit: indent
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> File chrome/browser/extensions/external_registry_loader_win.h (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
> chrome/browser/extensions/external_registry_loader_win.h:1: // Copyright (c)
> 2011 The Chromium Authors. All rights reserved.
> 2012
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/chrome_browser_...
> File chrome/chrome_browser_extensions.gypi (right):
> 
>
https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/chrome_browser_...
> chrome/chrome_browser_extensions.gypi:416:
> 'browser/extensions/external_pref_loader.h',
> Nit: order

Thanks for the thorough review, Joao. :) Regarding the extensions:: prefix in
comments, I agree that it is superfluous in classes that are (or will be)
themselves within the extensions:: namespace (I've removed these). However, I'd
like to keep it in comments in external classes, for two reasons:

 - It may actually increase readability if you are unfamiliar with the code in
question - it adds a description which otherwise may be missing. Even though we
all know that a manifest refers to an extensions manifest, the name "Manifest"
by itself is not as descriptive. Also PermissionSet, *Installer,
ComponentLoader, etc.

 - For consistency with other places in the code base. Easy example: search for
"// content::NotificationObserver" vs "// NotificationObserver" (typically to
indicate overridden methods). Codesearch lists 288 for the former, but 35 for
the latter. And that's with a class with which pretty much all of us are
familiar.

If you still think it should be removed, I'd be happy to do so - just wanted to
put in my two cents. :)

[Devlin, 2012-07-13 16:31:17]

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/default_apps.h (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/default_apps.h:56: 
On 2012/07/13 13:33:42, Joao da Silva wrote:
> Nit: remove this extra newline

Done.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/extension_service_unittest.cc (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/extension_service_unittest.cc:43: #include
"chrome/browser/extensions/external_pref_loader.h"
On 2012/07/13 13:33:42, Joao da Silva wrote:
> Nit: order

Done.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_policy_loader_unittest.cc (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_policy_loader_unittest.cc:13: #include
"chrome/browser/extensions/external_policy_loader.h"
On 2012/07/13 13:33:42, Joao da Silva wrote:
> Nit: order

Done.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_provider_impl.cc (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_provider_impl.cc:20: #include
"chrome/browser/extensions/external_pref_loader.h"
On 2012/07/13 13:33:42, Joao da Silva wrote:
> Nit: order

Done.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_provider_interface.h (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_provider_interface.h:55: const
ExternalProviderInterface* provider) = 0;
On 2012/07/13 13:33:42, Joao da Silva wrote:
> Nit: indent

Done.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/external_registry_loader_win.h (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/browser/extensi...
chrome/browser/extensions/external_registry_loader_win.h:1: // Copyright (c)
2011 The Chromium Authors. All rights reserved.
On 2012/07/13 13:33:42, Joao da Silva wrote:
> 2012

Done.

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/chrome_browser_...
File chrome/chrome_browser_extensions.gypi (right):

https://chromiumcodereview.appspot.com/10692168/diff/1/chrome/chrome_browser_...
chrome/chrome_browser_extensions.gypi:416:
'browser/extensions/external_pref_loader.h',
On 2012/07/13 13:33:42, Joao da Silva wrote:
> Nit: order

Done.

[not at google - send to devlin, 2012-07-16 07:18:09]

lgtm

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_policy_loader.cc (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_policy_loader.cc:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
2012

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_policy_loader.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_policy_loader.h:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
2012

[Joao da Silva, 2012-07-16 08:08:47]

A couple more nits; still lgtm

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_loader.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_loader.h:12: 
Nit: remove extra newline

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_loader.h:33: explicit ExternalLoader();
Nit: this doesn't have to be explicit

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_pref_loader.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_pref_loader.h:36: explicit
ExternalPrefLoader(int base_path_id, Options options);
Nit: doesn't have to be explicit

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/policy/app_p...
File chrome/browser/policy/app_pack_updater.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/policy/app_p...
chrome/browser/policy/app_pack_updater.h:25: class FilePath;
Nit: "base/file_path.h" is already included

[Devlin, 2012-07-16 15:40:30]

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_loader.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_loader.h:12: 
On 2012/07/16 08:08:47, Joao da Silva wrote:
> Nit: remove extra newline

Done.

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_loader.h:33: explicit ExternalLoader();
On 2012/07/16 08:08:47, Joao da Silva wrote:
> Nit: this doesn't have to be explicit

Done.

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_policy_loader.cc (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_policy_loader.cc:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
On 2012/07/16 07:18:10, kalman wrote:
> 2012

Done.

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_policy_loader.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_policy_loader.h:1: // Copyright (c) 2011 The
Chromium Authors. All rights reserved.
On 2012/07/16 07:18:10, kalman wrote:
> 2012

Done.

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
File chrome/browser/extensions/external_pref_loader.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/extensions/e...
chrome/browser/extensions/external_pref_loader.h:36: explicit
ExternalPrefLoader(int base_path_id, Options options);
On 2012/07/16 08:08:47, Joao da Silva wrote:
> Nit: doesn't have to be explicit

Done.

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/policy/app_p...
File chrome/browser/policy/app_pack_updater.h (right):

http://codereview.chromium.org/10692168/diff/9027/chrome/browser/policy/app_p...
chrome/browser/policy/app_pack_updater.h:25: class FilePath;
On 2012/07/16 08:08:47, Joao da Silva wrote:
> Nit: "base/file_path.h" is already included

Done.

[commit-bot: I haz the power, 2012-07-16 15:40:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/10692168/1...

[commit-bot: I haz the power, 2012-07-16 16:05:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/10692168/1...

[commit-bot: I haz the power, 2012-07-16 17:11:34]

Try job failure for 10692168-17004 (retry) on mac_rel for step "browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-07-16 17:45:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/10692168/2...

[commit-bot: I haz the power, 2012-07-16 19:03:29]

Change committed as 146847