Moved ExtensionPrefs and related into extensions namespace

Moved ExtensionPrefs and related into extensions namespace

Moved ExtensionPrefs, ExtensionPrefsScope, and TestExtensionPrefs into
extensions namespace.

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

[Devlin, 2012-07-05 19:24:12]

Refactor; please review when you get a chance:
asargent: chrome/browser/extensions
ben: chrome/browser/ui
ivankr: chrome/browser/protector

Thanks in advance.

[Ivan Korotkov, 2012-07-05 20:23:13]

protector/ LGTM

[Ben Goodger (Google), 2012-07-09 15:36:27]

Would you consider ExtensionPrefs browser-related? Or is it part of the
extension subsystem shared by extensions and apps?

Reason I ask, we are trying to consolidate namespaces within src/chrome and make
them more aligned with elsewhere in the codebase. Meaning that we tend to use a
chrome namespace first.

e.g. the search folk wanted a search namespace, which was fine, but I requested
they nest within the chrome namespace, so we don't create a toplevel namespace
that doesn't correspond to a toplevel dir.

so, I'm fine with you adding an extensions-specific namespace, but if it's for
code that will always live in src/chrome, I'm going to ask that it be nested
within a chrome namespace. (eventually we'll migrate all code in src/chrome into
a chrome namespace).

[Devlin, 2012-07-09 16:20:33]

On 2012/07/09 15:36:27, Ben Goodger (Google) wrote:
> Would you consider ExtensionPrefs browser-related? Or is it part of the
> extension subsystem shared by extensions and apps?
> 
> Reason I ask, we are trying to consolidate namespaces within src/chrome and
make
> them more aligned with elsewhere in the codebase. Meaning that we tend to use
a
> chrome namespace first.
> 
> e.g. the search folk wanted a search namespace, which was fine, but I
requested
> they nest within the chrome namespace, so we don't create a toplevel namespace
> that doesn't correspond to a toplevel dir.
> 
> so, I'm fine with you adding an extensions-specific namespace, but if it's for
> code that will always live in src/chrome, I'm going to ask that it be nested
> within a chrome namespace. (eventually we'll migrate all code in src/chrome
into
> a chrome namespace).

This doesn't add an extensions namespace; there is already an extensions
namespace in place and populated with many classes from
chrome/browser/extensions, chrome/common/extensions, and
chrome/renderer/extensions. The older classes from these areas were written
prior to the creation of the namespace, though, and are not in any namespace.
crbug.com/117261 is trying to reconcile all this code into the already-existing
extensions namespace.

I'm inclined to not create a chrome::extensions:: namespace until all extensions
code has been migrated to the existing extensions:: namespace. That way, we can
just nest everything in that namespace in chrome::. Additionally, if we begin
adding things to a chrome::extensions:: namespace now, the result would be
having three different namespace scopes for extensions-related classes (contrary
to this issue's goal of consolidating the code).

Does all that sound reasonable?

[Ben Goodger (Google), 2012-07-09 16:45:13]

OK. LGTM.


On Mon, Jul 9, 2012 at 9:20 AM, <rdevlin.cronin@chromium.org> wrote:

> On 2012/07/09 15:36:27, Ben Goodger (Google) wrote:
>
>> Would you consider ExtensionPrefs browser-related? Or is it part of the
>> extension subsystem shared by extensions and apps?
>>
>
>  Reason I ask, we are trying to consolidate namespaces within src/chrome
>> and
>>
> make
>
>> them more aligned with elsewhere in the codebase. Meaning that we tend to
>> use
>>
> a
>
>> chrome namespace first.
>>
>
>  e.g. the search folk wanted a search namespace, which was fine, but I
>>
> requested
>
>> they nest within the chrome namespace, so we don't create a toplevel
>> namespace
>> that doesn't correspond to a toplevel dir.
>>
>
>  so, I'm fine with you adding an extensions-specific namespace, but if
>> it's for
>> code that will always live in src/chrome, I'm going to ask that it be
>> nested
>> within a chrome namespace. (eventually we'll migrate all code in
>> src/chrome
>>
> into
>
>> a chrome namespace).
>>
>
> This doesn't add an extensions namespace; there is already an extensions
> namespace in place and populated with many classes from
> chrome/browser/extensions, chrome/common/extensions, and
> chrome/renderer/extensions. The older classes from these areas were written
> prior to the creation of the namespace, though, and are not in any
> namespace.
> crbug.com/117261 is trying to reconcile all this code into the
> already-existing
> extensions namespace.
>
> I'm inclined to not create a chrome::extensions:: namespace until all
> extensions
> code has been migrated to the existing extensions:: namespace. That way,
> we can
> just nest everything in that namespace in chrome::. Additionally, if we
> begin
> adding things to a chrome::extensions:: namespace now, the result would be
> having three different namespace scopes for extensions-related classes
> (contrary
> to this issue's goal of consolidating the code).
>
> Does all that sound reasonable?
>
>
https://chromiumcodereview.**appspot.com/10704023/<https://chromiumcodereview...
>

[asargent_no_longer_on_chrome, 2012-07-09 18:13:38]

LGTM w/ one question

https://chromiumcodereview.appspot.com/10704023/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/extension_pref_value_map.cc (right):

https://chromiumcodereview.appspot.com/10704023/diff/2001/chrome/browser/exte...
chrome/browser/extensions/extension_pref_value_map.cc:11: using
extensions::ExtensionPrefsScope;
do you need this "using" statement if you're prefixing the usages of the
ExtensionPrefsScope enum members below with "extensions::" ?

[Devlin, 2012-07-09 18:58:31]

https://chromiumcodereview.appspot.com/10704023/diff/2001/chrome/browser/exte...
File chrome/browser/extensions/extension_pref_value_map.cc (right):

https://chromiumcodereview.appspot.com/10704023/diff/2001/chrome/browser/exte...
chrome/browser/extensions/extension_pref_value_map.cc:11: using
extensions::ExtensionPrefsScope;
On 2012/07/09 18:13:38, Antony Sargent wrote:
> do you need this "using" statement if you're prefixing the usages of the
> ExtensionPrefsScope enum members below with "extensions::" ?
> 

This part was kind of annoying - the using is to avoid having to say
"extensions::ExtensionPrefsScope scope" whenever we have the scope as a type,
e.g. as a parameter to a function. It does not carry over to the enum members
themselves, so we must still prefix those with extensions::.

[asargent_no_longer_on_chrome, 2012-07-09 19:40:53]

Ok, sounds good.

[commit-bot: I haz the power, 2012-07-09 20:41:26]

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

[commit-bot: I haz the power, 2012-07-09 22:32:27]

Failed to apply patch for chrome/browser/extensions/extension_prefs.cc:
While running patch -p1 --forward --force;
patching file chrome/browser/extensions/extension_prefs.cc
Hunk #1 FAILED at 25.
Hunk #2 succeeded at 277 (offset -2 lines).
Hunk #3 succeeded at 644 (offset -27 lines).
Hunk #4 succeeded at 1025 (offset -27 lines).
Hunk #5 succeeded at 1039 (offset -27 lines).
Hunk #6 succeeded at 1289 (offset -27 lines).
Hunk #7 succeeded at 1311 (offset -27 lines).
Hunk #8 succeeded at 1944 (offset -27 lines).
1 out of 8 hunks FAILED -- saving rejects to file
chrome/browser/extensions/extension_prefs.cc.rej

[commit-bot: I haz the power, 2012-07-10 15:58:06]

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

[commit-bot: I haz the power, 2012-07-10 17:21:25]

Change committed as 145898