Switch platform apps from a declarative launch container to handling an onLaunched event.

chrome/common/extensions/extension.cc

Index: chrome/common/extensions/extension.cc
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 9e2ac3b960b9dda86cc5ff5029a76ffbf157d569..4c8f401c6b75c2e2f16de411b39cc4a316349f93 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -479,11 +479,17 @@ scoped_refptr<Extension> Extension::Create(const FilePath& path,
     return NULL;
   }
 
-  if (extension->is_platform_app() &&
-      !CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnablePlatformApps)) {
-    *utf8_error = errors::kPlatformAppFlagRequired;
-    return NULL;
+  if (extension->is_platform_app()) {
+    if (!CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kEnablePlatformApps)) {
+      *utf8_error = errors::kPlatformAppFlagRequired;
+      return NULL;
+    }
+
+    if (!extension->has_background_page()) {
+      *utf8_error = errors::kBackgroundRequiredForPlatformApps;
+      return NULL;
+    }
   }
 
   return extension;
@@ -1265,6 +1271,11 @@ bool Extension::LoadExtent(const char* key,
 bool Extension::LoadLaunchURL(string16* error) {
   Value* temp = NULL;
 
+  if (is_platform_app() && manifest_->Get(keys::kLaunch, &temp)) {
+      *error = ASCIIToUTF16(errors::kLaunchNotAllowedForPlatformApps);
+      return false;
+  }

[miket_OOO, 2012/04/06 18:05:01]

I wish there were a single point in this file where we could say that if you get
here, the extension is valid. Instead we have a lot of condition checking all
over the place, many of which can determine that the extension is fatally
flawed.

If you have an idea for centralizing these tests, this might be a nice time to
do it. But if not, let's do it some other time.

[Aaron Boodman, 2012/04/06 18:59:01]

It should be possible to extend the feature system to support subkeys ... I just
had hoped we wouldn't need it :-(.

[Mihai Parparita -not on Chrome, 2012/04/06 22:02:59]

That seems pretty doable if we invert the loop in Manifest::ValidateManifest to
loop over the feature keys instead of over the manifest keys (then there could
be a separate app.launch key). I've added a TODO and filed
http://crbug.com/122459.

+
   // launch URL can be either local (to chrome-extension:// root) or an absolute
   // web URL.
   if (manifest_->Get(keys::kLaunchLocalPath, &temp)) {
@@ -1316,7 +1327,7 @@ bool Extension::LoadLaunchURL(string16* error) {
     }
 
     launch_web_url_ = launch_url;
-  } else if (is_app()) {
+  } else if (is_packaged_app() || is_hosted_app()) {
     *error = ASCIIToUTF16(errors::kLaunchURLRequired);
     return false;
   }
@@ -1381,9 +1392,7 @@ bool Extension::LoadLaunchContainer(string16* error) {
     return false;
   }
 
-  if (launch_container_string == values::kLaunchContainerShell) {
-    launch_container_ = extension_misc::LAUNCH_SHELL;
-  } else if (launch_container_string == values::kLaunchContainerPanel) {
+  if (launch_container_string == values::kLaunchContainerPanel) {
     launch_container_ = extension_misc::LAUNCH_PANEL;
   } else if (launch_container_string == values::kLaunchContainerTab) {
     launch_container_ = extension_misc::LAUNCH_TAB;
@@ -1394,8 +1403,7 @@ bool Extension::LoadLaunchContainer(string16* error) {
 
   bool can_specify_initial_size =
       launch_container_ == extension_misc::LAUNCH_PANEL ||
-      launch_container_ == extension_misc::LAUNCH_WINDOW ||
-      launch_container_ == extension_misc::LAUNCH_SHELL;
+      launch_container_ == extension_misc::LAUNCH_WINDOW;
 
   // Validate the container width if present.
   if (!ReadLaunchDimension(manifest_,
@@ -1413,72 +1421,6 @@ bool Extension::LoadLaunchContainer(string16* error) {
                            error))
       return false;
 
-  bool can_specify_size_range =
-      launch_container_ == extension_misc::LAUNCH_SHELL;
-
-  // Validate min size if present.
-  if (!ReadLaunchDimension(manifest_,
-                           keys::kLaunchMinWidth,
-                           &launch_min_width_,
-                           can_specify_size_range,
-                           error))
-      return false;
-  if (!ReadLaunchDimension(manifest_,
-                           keys::kLaunchMinHeight,
-                           &launch_min_height_,
-                           can_specify_size_range,
-                           error))
-      return false;
-  if (!ReadLaunchDimension(manifest_,
-                           keys::kLaunchMaxWidth,
-                           &launch_max_width_,
-                           can_specify_size_range,
-                           error))
-      return false;
-  if (!ReadLaunchDimension(manifest_,
-                           keys::kLaunchMaxHeight,
-                           &launch_max_height_,
-                           can_specify_size_range,
-                           error))
-      return false;
-
-  if (launch_container_ == extension_misc::LAUNCH_SHELL) {
-    if (!manifest_->Get(keys::kLaunchWidth, &temp)) {
-      *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
-          errors::kInvalidLaunchValue,
-          keys::kLaunchWidth);
-      return false;
-    }
-    if (!manifest_->Get(keys::kLaunchHeight, &temp)) {
-      *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
-          errors::kInvalidLaunchValue,
-          keys::kLaunchHeight);
-      return false;
-    }
-    if (launch_max_width_ > 0 && launch_max_width_ < launch_min_width_) {
-      *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
-          errors::kInvalidLaunchValue,
-          keys::kLaunchMaxWidth);
-      return false;
-    }
-    if (launch_max_height_ > 0 && launch_max_height_ < launch_min_height_) {
-      *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
-          errors::kInvalidLaunchValue,
-          keys::kLaunchMaxHeight);
-      return false;
-    }
-  }
-
-  if (is_platform_app()) {
-    if (launch_container_ != extension_misc::LAUNCH_SHELL) {
-      *error = ASCIIToUTF16(errors::kInvalidLaunchContainerForPlatform);
-      return false;
-    }
-  } else if (launch_container_ == extension_misc::LAUNCH_SHELL) {
-    *error = ASCIIToUTF16(errors::kInvalidLaunchContainerForNonPlatform);
-    return false;
-  }

[miket_OOO, 2012/04/06 18:05:01]

Great!

-
   return true;
 }
 
@@ -2868,10 +2810,6 @@ Extension::Extension(const FilePath& path,
       launch_container_(extension_misc::LAUNCH_TAB),
       launch_width_(0),
       launch_height_(0),
-      launch_min_width_(0),
-      launch_min_height_(0),
-      launch_max_width_(0),
-      launch_max_height_(0),
       wants_file_access_(false),
       creation_flags_(0) {
   DCHECK(path.empty() || path.IsAbsolute());