Add the "options_ui" extension manifest field.

extensions/common/manifest_handlers/options_page_info.cc

Index: extensions/common/manifest_handlers/options_page_info.cc
diff --git a/extensions/common/manifest_handlers/options_page_info.cc b/extensions/common/manifest_handlers/options_page_info.cc
new file mode 100644
index 0000000000000000000000000000000000000000..481f614537ce877ea051ea919144ea5b53537183
--- /dev/null
+++ b/extensions/common/manifest_handlers/options_page_info.cc
@@ -0,0 +1,239 @@
+// Copyright 2014 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 "extensions/common/manifest_handlers/options_page_info.h"
+
+#include "base/file_util.h"
+#include "base/lazy_instance.h"

[Devlin, 2014/09/02 21:42:29]

need?

[ericzeng, 2014/09/03 17:52:14]

Not anymore

+#include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
+#include "extensions/common/api/extensions_manifest_types.h"
+#include "extensions/common/error_utils.h"
+#include "extensions/common/feature_switch.h"
+#include "extensions/common/file_util.h"
+#include "extensions/common/manifest_constants.h"
+#include "extensions/strings/grit/extensions_strings.h"
+#include "ui/base/l10n/l10n_util.h"
+
+using base::ASCIIToUTF16;
+using base::DictionaryValue;
+
+namespace extensions {
+
+namespace keys = manifest_keys;
+namespace values = manifest_values;
+namespace errors = manifest_errors;
+
+using core_api::extensions_manifest_types::OptionsUI;
+
+namespace {
+
+OptionsPageInfo* GetOptionsPageInfo(const Extension* extension) {
+  return static_cast<OptionsPageInfo*>(
+      extension->GetManifestData(keys::kOptionsUI));
+}
+
+// Parses |url_string| into a GURL |result| if it is a valid options page for
+// this app/extension. If not, it returns the reason in |error|. Because this
+// handles URLs for both "options_page" and "options_ui.page", the name of the
+// manifest field must be provided in |manifest_field_name|.
+bool ParseOptionsUrl(Extension* extension,
+                     const std::string& url_string,
+                     const std::string& manifest_field_name,
+                     base::string16* error,
+                     GURL* result) {
+  if (extension->is_hosted_app()) {
+    // Hosted apps require an absolute URL.
+    GURL options_url(url_string);
+    if (!options_url.is_valid() || !options_url.SchemeIsHTTPOrHTTPS()) {
+      *error = base::ASCIIToUTF16(errors::kInvalidOptionsPageInHostedApp);
+      return false;
+    }
+    *result = options_url;
+    return true;
+  }
+
+  // Otherwise the options URL should be inside the extension.
+  if (GURL(url_string).is_valid()) {
+    *error = base::ASCIIToUTF16(errors::kInvalidOptionsPageExpectUrlInPackage);
+    return false;
+  }
+
+  GURL resource_url = extension->GetResourceURL(url_string);
+  if (!resource_url.is_valid()) {
+    *error = ErrorUtils::FormatErrorMessageUTF16(errors::kInvalidOptionsPage,
+                                                 manifest_field_name);
+    return false;
+  }
+  *result = resource_url;
+  return true;
+}
+
+}  // namespace
+
+OptionsPageInfo::OptionsPageInfo(const GURL& options_url,
+                                 bool chrome_styles,
+                                 bool open_in_tab)
+    : options_url_(options_url),
+      chrome_styles_(chrome_styles),
+      open_in_tab_(open_in_tab) {
+}
+
+OptionsPageInfo::~OptionsPageInfo() {
+}
+
+// static
+const GURL& OptionsPageInfo::GetOptionsPage(const Extension* extension) {
+  OptionsPageInfo* info = GetOptionsPageInfo(extension);
+  if (!info) {

[Devlin, 2014/09/02 21:42:29]

nit: no brackets

[ericzeng, 2014/09/03 17:52:14]

Done.

+    return GURL::EmptyGURL();

[Devlin, 2014/09/02 21:42:30]

better yet,
return info ? info->options_url_ : GURL::EmptyGURL();

Might be worth briefly combing through the uses, and see if EmptyGURL is
actually necessary (instead of just GURL).  WDYT, Yoyo?

[Yoyo Zhou, 2014/09/02 21:54:35]

Hmm,
  // This object is for callers
  // who return references but don't have anything to return in some cases.

Are you referring to something else?

[Devlin, 2014/09/02 22:08:19]

Nope, that's it.  Just that, IIRC, C++ extends the life of a temporary to the
current scope, so something like:

const GURL& GetURL() {
  return GURL();
}

void Foo() {
  const GURL& url = GetURL();
  <do_stuff_with_url>
}

is safe, even though |url| doesn't really have an owning object.

That, coupled with the impressive warnings on the similar EmptyString()
functions, make me wonder if we should be returning EmptyGURL() vs GURL() here. 
Not sure if the risk of someone actually keeping around a GURL reference is
better or worse than the extra overhead for the EmptyGURL() call.

[Yoyo Zhou, 2014/09/02 22:10:29]

Even so, none of the scary warnings in EmptyString apply here; this is exactly
the situation where we would return EmptyString if it were a string.

[Devlin, 2014/09/02 22:14:57]

Mmkay, just confirming. :)

[not at google - send to devlin, 2014/09/03 16:11:29]

Yes this is exactly what GURL::EmptyGURL is for, it allows the caller to assign
to a const& rather than needing to take a copy.

+  }
+  return info->options_url_;
+}
+
+// static
+bool OptionsPageInfo::HasOptionsPage(const Extension* extension) {
+  return !OptionsPageInfo::GetOptionsPage(extension).is_empty();
+}
+
+// static
+bool OptionsPageInfo::ShouldUseChromeStyle(const Extension* extension) {
+  OptionsPageInfo* info = GetOptionsPageInfo(extension);
+  return info && info->chrome_styles_;
+}
+
+// static
+bool OptionsPageInfo::ShouldOpenInTab(const Extension* extension) {
+  OptionsPageInfo* info = GetOptionsPageInfo(extension);
+  return info && info->open_in_tab_;
+}
+
+scoped_ptr<OptionsPageInfo> OptionsPageInfo::Create(
+    Extension* extension,
+    const base::Value* options_ui_value,
+    const std::string& options_page_string,
+    std::vector<InstallWarning>* install_warnings,
+    base::string16* error) {
+  GURL options_page;
+  GURL options_ui_page;
+  bool chrome_style = false;
+  bool open_in_tab = !FeatureSwitch::embedded_extension_options()->IsEnabled();

[Devlin, 2014/09/02 21:42:30]

Do we want to default this to true with the switch on?

[ericzeng, 2014/09/03 17:52:14]

I believe so, Ben and I discussed that we wanted chrome styles to be opt-in in
all cases, but the popup to be opt-out. 

+
+  // Parse the options_ui object.
+  if (options_ui_value &&
+      FeatureSwitch::embedded_extension_options()->IsEnabled()) {
+    base::string16 options_ui_error;
+
+    scoped_ptr<OptionsUI> options_ui =
+        OptionsUI::FromValue(*options_ui_value, &options_ui_error);
+    if (options_ui_error.empty()) {

[Devlin, 2014/09/02 21:42:30]

Do you mean if (!empty())?

[ericzeng, 2014/09/03 17:52:14]

Oops, yes

+      // OptionsUI::FromValue populates |error| both when there are
+      // errors (in which case |options_ui| will be NULL) and warnings
+      // (in which case |options_ui| will be valid). Either way, show it
+      // as an install warning.
+      install_warnings->push_back(
+          InstallWarning(base::UTF16ToASCII(options_ui_error)));
+    }
+
+    if (options_ui) {
+      base::string16 options_parse_error;
+      if (!ParseOptionsUrl(extension,
+                           options_ui->page,
+                           keys::kOptionsUI,
+                           &options_parse_error,
+                           &options_ui_page)) {
+        install_warnings->push_back(
+            InstallWarning(base::UTF16ToASCII(options_parse_error)));
+      }
+      chrome_style =
+          options_ui->chrome_style.get() && *options_ui->chrome_style;
+      open_in_tab = options_ui->open_in_tab.get() && *options_ui->open_in_tab;
+    }
+  }
+
+  // Parse the legacy options_page entry.
+  if (!options_page_string.empty()) {

[Devlin, 2014/09/02 21:42:30]

Why not instead do something like:
GURL options_page;
if (options_ui_value && switch->enabled())
  options_page = <parse>

if (!options_page.is_valid())
  options_page = <parse>

Then options page will already be what you want, and you only parse the legacy
if you need to.

[ericzeng, 2014/09/03 17:52:14]

Done.

+    if (!ParseOptionsUrl(extension,
+                         options_page_string,
+                         keys::kOptionsPage,
+                         error,
+                         &options_page)) {
+      return scoped_ptr<OptionsPageInfo>();
+    }
+  }
+
+  // Prefer the options_ui.page url over the options_page url.
+  GURL options_url;
+  if (options_ui_page.is_valid())
+    options_url = options_ui_page;
+  else
+    options_url = options_page;
+
+  return make_scoped_ptr(
+      new OptionsPageInfo(options_url, chrome_style, open_in_tab));
+}
+
+OptionsPageManifestHandler::OptionsPageManifestHandler() {
+}
+
+OptionsPageManifestHandler::~OptionsPageManifestHandler() {
+}
+
+bool OptionsPageManifestHandler::Parse(Extension* extension,
+                                       base::string16* error) {
+  std::vector<InstallWarning> install_warnings;
+  const Manifest* manifest = extension->manifest();
+
+  std::string options_page_string;
+  if (manifest->HasPath(keys::kOptionsPage) &&
+      !manifest->GetString(keys::kOptionsPage, &options_page_string)) {
+    *error = ErrorUtils::FormatErrorMessageUTF16(errors::kInvalidOptionsPage,
+                                                 keys::kOptionsPage);
+    return false;
+  }
+
+  const base::Value* options_ui_value = NULL;
+  ignore_result(manifest->Get(keys::kOptionsUI, &options_ui_value));
+
+  scoped_ptr<OptionsPageInfo> info =
+      OptionsPageInfo::Create(extension,
+                              options_ui_value,
+                              options_page_string,
+                              &install_warnings,
+                              error);
+  if (!info)
+    return false;
+
+  extension->AddInstallWarnings(install_warnings);
+  extension->SetManifestData(keys::kOptionsUI, info.release());
+  return true;
+}
+
+bool OptionsPageManifestHandler::Validate(
+    const Extension* extension,
+    std::string* error,
+    std::vector<InstallWarning>* warnings) const {
+  // Validate path to the options page.  Don't check the URL for hosted apps,
+  // because they are expected to refer to an external URL.
+  if (!OptionsPageInfo::HasOptionsPage(extension) || extension->is_hosted_app())
+    return true;
+
+  base::FilePath options_path =
+      extensions::file_util::ExtensionURLToRelativeFilePath(

[Devlin, 2014/09/02 21:42:30]

remove extensions:: prefix.

[ericzeng, 2014/09/03 17:52:14]

Done.

+          OptionsPageInfo::GetOptionsPage(extension));
+  base::FilePath path = extension->GetResource(options_path).GetFilePath();
+  if (path.empty() || !base::PathExists(path)) {
+    *error = l10n_util::GetStringFUTF8(IDS_EXTENSION_LOAD_OPTIONS_PAGE_FAILED,
+                                       options_path.LossyDisplayName());
+    return false;
+  }
+  return true;
+}
+
+const std::vector<std::string> OptionsPageManifestHandler::Keys() const {
+  static const char* keys[] = {keys::kOptionsPage, keys::kOptionsUI};
+  return std::vector<std::string>(keys, keys + arraysize(keys));
+}
+
+}  // namespace extensions