Move Extension Location and Type enums to Manifest, and move InstallWarning to its own file.

Move Extension Location and Type enums to Manifest, and move InstallWarning to its own file.

Reverses the dependency between Extension and Manifest.
Part 1 of moving Manifest to top-level extensions.

BUG=162530
TBR=ben@chromium.org


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

[Yoyo Zhou, 2013-01-29 04:05:28]

Sorry, there's a lot of files here, but the changes are largely mindless.

[Jeffrey Yasskin, 2013-01-29 22:54:07]

Holy changelist, batman!

https://codereview.chromium.org/12093036/diff/5001/chrome/browser/extensions/...
File chrome/browser/extensions/external_provider_impl.h (right):

https://codereview.chromium.org/12093036/diff/5001/chrome/browser/extensions/...
chrome/browser/extensions/external_provider_impl.h:24: class Manifest;
If you're using Manifest::anything, a forward declaration isn't enough.
Something in the #includes must already be including manifest.h (and
extension.h), so just include it directly.

https://codereview.chromium.org/12093036/diff/5001/chrome/browser/extensions/...
File chrome/browser/extensions/external_provider_interface.h (right):

https://codereview.chromium.org/12093036/diff/5001/chrome/browser/extensions/...
chrome/browser/extensions/external_provider_interface.h:12: #include
"googleurl/src/gurl.h"
Strictly, GURL could be a forward declaration here.  Using the #include is
somewhat better if users take advantage of any implicit conversions.

https://codereview.chromium.org/12093036/diff/5001/chrome/browser/extensions/...
File chrome/browser/extensions/tab_helper.h (right):

https://codereview.chromium.org/12093036/diff/5001/chrome/browser/extensions/...
chrome/browser/extensions/tab_helper.h:132: //
Manifest::EXTENSION_ICON_SMALLISH).
This enum is still in Extension.

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/e...
File chrome/common/extensions/extension.h (right):

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/e...
chrome/common/extensions/extension.h:253: // See Type definition above.
The Type definition is no longer "above".

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/e...
chrome/common/extensions/extension.h:1003: void PrintTo(const InstallWarning&,
::std::ostream* os);
Move this to install_warning.h so that all users can take advantage of it.

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/m...
File chrome/common/extensions/manifest.cc (right):

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/m...
chrome/common/extensions/manifest.cc:97: // TODO(yoz): remove extraneous
Manifest::
Why can't you do it now?

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/m...
File chrome/common/extensions/manifest.h (right):

https://codereview.chromium.org/12093036/diff/5001/chrome/common/extensions/m...
chrome/common/extensions/manifest.h:47: enum Type {
s/Type/ExtensionType/? Or is this really a "manifest type"? The second does seem
plausible.

https://codereview.chromium.org/12093036/diff/5001/extensions/common/install_...
File extensions/common/install_warning.h (right):

https://codereview.chromium.org/12093036/diff/5001/extensions/common/install_...
extensions/common/install_warning.h:14: typedef std::vector<InstallWarning>
Vector;
Is this typedef worth having? std::vector<InstallWarning> isn't much longer than
InstallWarning::Vector at all, and it's more obvious what type you get.

[Yoyo Zhou, 2013-01-30 01:16:00]

Please look again. Patchset 4 are the changes to the original files; patchset 5
rebases on trunk and changes manifest_handler accordingly.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/browser/exte...
File chrome/browser/extensions/external_provider_impl.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/browser/exte...
chrome/browser/extensions/external_provider_impl.h:24: class Manifest;
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> If you're using Manifest::anything, a forward declaration isn't enough.
> Something in the #includes must already be including manifest.h (and
> extension.h), so just include it directly.

Done.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/browser/exte...
File chrome/browser/extensions/external_provider_interface.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/browser/exte...
chrome/browser/extensions/external_provider_interface.h:12: #include
"googleurl/src/gurl.h"
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> Strictly, GURL could be a forward declaration here.  Using the #include is
> somewhat better if users take advantage of any implicit conversions.

Done.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/browser/exte...
File chrome/browser/extensions/tab_helper.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/browser/exte...
chrome/browser/extensions/tab_helper.h:132: //
Manifest::EXTENSION_ICON_SMALLISH).
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> This enum is still in Extension.

Actually it's not there either.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
File chrome/common/extensions/extension.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
chrome/common/extensions/extension.h:253: // See Type definition above.
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> The Type definition is no longer "above".

Done.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
chrome/common/extensions/extension.h:1003: void PrintTo(const InstallWarning&,
::std::ostream* os);
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> Move this to install_warning.h so that all users can take advantage of it.

It makes more sense there, anyway. Added install_warning.cc.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
File chrome/common/extensions/manifest.cc (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
chrome/common/extensions/manifest.cc:97: // TODO(yoz): remove extraneous
Manifest::
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> Why can't you do it now?

I left this here so I would remember to actually do it.

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
File chrome/common/extensions/manifest.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
chrome/common/extensions/manifest.h:47: enum Type {
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> s/Type/ExtensionType/? Or is this really a "manifest type"? The second does
seem
> plausible.

It's an "extension type" so long as an extension is a type of extension. =)

Until we come up with a good word for things that have manifests and can be one
of these types, I think "manifest type" is okay.

https://chromiumcodereview.appspot.com/12093036/diff/5001/extensions/common/i...
File extensions/common/install_warning.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/extensions/common/i...
extensions/common/install_warning.h:14: typedef std::vector<InstallWarning>
Vector;
On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> Is this typedef worth having? std::vector<InstallWarning> isn't much longer
than
> InstallWarning::Vector at all, and it's more obvious what type you get.

Might as well remove it.

[Jeffrey Yasskin, 2013-01-30 01:28:41]

lgtm

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
File chrome/common/extensions/manifest.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/5001/chrome/common/exten...
chrome/common/extensions/manifest.h:47: enum Type {
On 2013/01/30 01:16:01, Yoyo Zhou wrote:
> On 2013/01/29 22:54:07, Jeffrey Yasskin wrote:
> > s/Type/ExtensionType/? Or is this really a "manifest type"? The second does
> seem
> > plausible.
> 
> It's an "extension type" so long as an extension is a type of extension. =)
> 
> Until we come up with a good word for things that have manifests and can be
one
> of these types, I think "manifest type" is okay.

SGTM

https://chromiumcodereview.appspot.com/12093036/diff/11002/extensions/common/...
File extensions/common/install_warning.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/11002/extensions/common/...
extensions/common/install_warning.h:30: void PrintTo(const InstallWarning&,
::std::ostream* os);
This needs either #include <iosfwd> or #include <ostream>

[Yoyo Zhou, 2013-01-30 20:33:08]

TBRing ben because this refactoring makes trivial changes everywhere

https://chromiumcodereview.appspot.com/12093036/diff/11002/extensions/common/...
File extensions/common/install_warning.h (right):

https://chromiumcodereview.appspot.com/12093036/diff/11002/extensions/common/...
extensions/common/install_warning.h:30: void PrintTo(const InstallWarning&,
::std::ostream* os);
On 2013/01/30 01:28:42, Jeffrey Yasskin wrote:
> This needs either #include <iosfwd> or #include <ostream>

Moved ostream include from .cc to here.

[commit-bot: I haz the power, 2013-01-30 20:40:03]

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

[commit-bot: I haz the power, 2013-01-30 21:04:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on ios_dbg_simulator.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...
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.

[commit-bot: I haz the power, 2013-01-30 21:19:16]

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

[commit-bot: I haz the power, 2013-01-30 23:06:35]

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

[commit-bot: I haz the power, 2013-01-31 08:41:45]

Change committed as 179828