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

chrome/common/extensions/extension.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/common/extensions/extension.h"
 
 #include <algorithm>
 
 #include "base/base64.h"
 #include "base/basictypes.h"
@@ skipping 461 matching lines @@
     *utf8_error = UTF16ToUTF8(error);
     return NULL;
   }
 
   scoped_refptr<Extension> extension = new Extension(path, manifest.Pass());
   if (!extension->InitFromValue(flags, &error)) {
     *utf8_error = UTF16ToUTF8(error);
     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;
 }
 
 // static
 Extension::Location Extension::GetHigherPriorityLocation(
     Extension::Location loc1, Extension::Location loc2) {
   if (loc1 == loc2)
     return loc1;
@@ skipping 761 matching lines @@
 
     extent->AddPattern(pattern);
   }
 
   return true;
 }
 
 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)) {
     if (manifest_->Get(keys::kLaunchWebURL, NULL)) {
       *error = ASCIIToUTF16(errors::kLaunchPathAndURLAreExclusive);
       return false;
     }
 
     if (manifest_->Get(keys::kWebURLs, NULL)) {
       *error = ASCIIToUTF16(errors::kLaunchPathAndExtentAreExclusive);
@@ skipping 31 matching lines @@
     GURL url(launch_url);
     URLPattern pattern(kValidWebExtentSchemes);
     if (!url.is_valid() || !pattern.SetScheme(url.scheme())) {
       *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
           errors::kInvalidLaunchValue,
           keys::kLaunchWebURL);
       return false;
     }
 
     launch_web_url_ = launch_url;
-  } else if (is_app()) {
+  } else if (is_packaged_app() || is_hosted_app()) {
     *error = ASCIIToUTF16(errors::kLaunchURLRequired);
     return false;
   }
 
   // If there is no extent, we default the extent based on the launch URL.
   if (web_extent().is_empty() && !launch_web_url().empty()) {
     GURL launch_url(launch_web_url());
     URLPattern pattern(kValidWebExtentSchemes);
     if (!pattern.SetScheme("*")) {
       *error = ExtensionErrorUtils::FormatErrorMessageUTF16(
@@ skipping 44 matching lines @@
   Value* temp = NULL;
   if (!manifest_->Get(keys::kLaunchContainer, &temp))
     return true;
 
   std::string launch_container_string;
   if (!temp->GetAsString(&launch_container_string)) {
     *error = ASCIIToUTF16(errors::kInvalidLaunchContainer);
     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;
   } else {
     *error = ASCIIToUTF16(errors::kInvalidLaunchContainer);
     return false;
   }
 
   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_,
                            keys::kLaunchWidth,
                            &launch_width_,
                            can_specify_initial_size,
                            error))
       return false;
 
   // Validate container height if present.
   if (!ReadLaunchDimension(manifest_,
                            keys::kLaunchHeight,
                            &launch_height_,
                            can_specify_initial_size,
                            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;
 }
 
 bool Extension::LoadSharedFeatures(
     const ExtensionAPIPermissionSet& api_permissions,
     string16* error) {
   if (!LoadDescription(error) ||
       !LoadHomepageURL(error) ||
       !LoadUpdateURL(error) ||
       !LoadIcons(error) ||
@@ skipping 1369 matching lines @@
       incognito_split_mode_(false),
       offline_enabled_(false),
       converted_from_user_script_(false),
       background_page_is_transient_(false),
       allow_background_js_access_(true),
       manifest_(manifest.release()),
       is_storage_isolated_(false),
       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());
   path_ = MaybeNormalizePath(path);
 }
 
 Extension::~Extension() {
   if (manifest_)
     delete manifest_;
 }
@@ skipping 820 matching lines @@
     already_disabled(false),
     extension(extension) {}
 
 UpdatedExtensionPermissionsInfo::UpdatedExtensionPermissionsInfo(
     const Extension* extension,
     const ExtensionPermissionSet* permissions,
     Reason reason)
     : reason(reason),
       extension(extension),
       permissions(permissions) {}