Move ".crx"/".pem" constants and extension_filenames constants into extensions/common/constants.cc.

Move ".crx"/".pem" constants and extension_filenames constants into extensions/common/constants.cc.

Continuation of cleanup of chrome/ includes in extension.cc.

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

[Matt Perry, 2013-07-18 23:01:52]

What do you think about dropping the "filenames" namespace, and just leaving
everything in "extensions"? Reasons:
- The desired style for chrome is to only have namespaces for toplevel modules
(which I realize we violate all the time).
- These are just constants. The fact that they are related to filenames isn't
necessarily important.
- It's simpler.

https://chromiumcodereview.appspot.com/19547009/diff/2001/extensions/common/c...
File extensions/common/constants.h (right):

https://chromiumcodereview.appspot.com/19547009/diff/2001/extensions/common/c...
extensions/common/constants.h:58: bool IsExtension(const base::FilePath&
file_name);
This is not a constant. It doesn't feel like it belongs here.

Also, as long as you're touching this code, it could use a more descriptive
name, like HasExtensionFilename.

[Yoyo Zhou, 2013-07-19 22:07:44]

Sure, got rid of extension_filenames and extensions::filenames namespaces.

https://chromiumcodereview.appspot.com/19547009/diff/2001/extensions/common/c...
File extensions/common/constants.h (right):

https://chromiumcodereview.appspot.com/19547009/diff/2001/extensions/common/c...
extensions/common/constants.h:58: bool IsExtension(const base::FilePath&
file_name);
On 2013/07/18 23:01:52, Matt Perry wrote:
> This is not a constant. It doesn't feel like it belongs here.
> 
> Also, as long as you're touching this code, it could use a more descriptive
> name, like HasExtensionFilename.

extension_constants.cc has a bunch of helper functions like this, which is why I
thought it was okay. Anyway, this was only used in 2 places, and we don't have
one for .pem files, so I just removed it.

[Matt Perry, 2013-07-19 22:29:11]

LGTM

https://chromiumcodereview.appspot.com/19547009/diff/10001/chrome/browser/ext...
File chrome/browser/extensions/sandboxed_unpacker.cc (right):

https://chromiumcodereview.appspot.com/19547009/diff/10001/chrome/browser/ext...
chrome/browser/extensions/sandboxed_unpacker.cc:27: // #include
"chrome/common/extensions/extension_constants.h"
remove

[Yoyo Zhou, 2013-07-19 22:50:03]

https://chromiumcodereview.appspot.com/19547009/diff/10001/chrome/browser/ext...
File chrome/browser/extensions/sandboxed_unpacker.cc (right):

https://chromiumcodereview.appspot.com/19547009/diff/10001/chrome/browser/ext...
chrome/browser/extensions/sandboxed_unpacker.cc:27: // #include
"chrome/common/extensions/extension_constants.h"
On 2013/07/19 22:29:11, Matt Perry wrote:
> remove

Oops, done.

[commit-bot: I haz the power, 2013-07-19 22:50:42]

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

[commit-bot: I haz the power, 2013-07-19 23:14:50]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[Yoyo Zhou, 2013-07-19 23:16:55]

TBR=ben@chromium.org for simple refactoring

[commit-bot: I haz the power, 2013-07-19 23:19:49]

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

[commit-bot: I haz the power, 2013-07-20 01:57:55]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Yoyo Zhou, 2013-07-23 01:52:43]

+gab: could you check that the change to chrome/installer/setup/DEPS is okay?

[gab, 2013-07-23 15:52:19]

This also makes it so that the 'setup' target should depend on that new
extensions target.

I believe we usually make a separate target for constants, e.g.,
'extension_constants'; other targets typically only need to include the
constants and shouldn't have to depend on all of 'extensions'.

In the same manner I think the DEPS file should only allow inclusion from
extensions/common/constants.h

[Yoyo Zhou, 2013-07-23 19:21:02]

On 2013/07/23 15:52:19, gab wrote:
> This also makes it so that the 'setup' target should depend on that new
> extensions target.
> 
> I believe we usually make a separate target for constants, e.g.,
> 'extension_constants'; other targets typically only need to include the
> constants and shouldn't have to depend on all of 'extensions'.
> 
> In the same manner I think the DEPS file should only allow inclusion from
> extensions/common/constants.h

We don't have separate gyp targets for anything extensions yet, so I added it to
the common_constants target. Also changed DEPS as you suggested.

[gab, 2013-07-23 19:38:14]

lgtm if others are ok with this temporary gyp change for the common_constants
target

https://codereview.chromium.org/19547009/diff/65001/chrome/common_constants.gyp
File chrome/common_constants.gyp (right):

https://codereview.chromium.org/19547009/diff/65001/chrome/common_constants.g...
chrome/common_constants.gyp:16: '../extensions/common/constants.cc',
Add a TODO to move this to an 'extension_constants' target.

[Yoyo Zhou, 2013-07-23 20:31:00]

+thestig: please review changes to common_constants.gyp

[Lei Zhang, 2013-07-23 20:33:32]

common_constant.gyp lgtm, considering there's no .gyp file anywhere in
src/extensions.

[commit-bot: I haz the power, 2013-07-23 20:37:51]

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

[commit-bot: I haz the power, 2013-07-23 23:17:05]

Change committed as 213260