Webview tag creation should be using storage partitions.

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index b765d7184df828fbcd03fe075ff277c74ff88d96..b09dd281597dd229cd2e3c047270040cdf29d0b6 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/path_service.h"
+#include "base/string_split.h"
 #include "base/string_tokenizer.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/breakpad_mac.h"
@@ -100,6 +101,7 @@
 #include "content/public/common/content_descriptors.h"
 #include "grit/generated_resources.h"
 #include "grit/ui_resources.h"
+#include "net/base/escape.h"
 #include "net/base/ssl_cert_request_info.h"
 #include "net/cookies/canonical_cookie.h"
 #include "net/cookies/cookie_options.h"
@@ -468,61 +470,119 @@ content::WebContentsView*
   return NULL;
 }
 
-std::string ChromeContentBrowserClient::GetStoragePartitionIdForChildProcess(
-    content::BrowserContext* browser_context,
-    int child_process_id) {
-  const Extension* extension = NULL;
-  Profile* profile = Profile::FromBrowserContext(browser_context);
-  ExtensionService* extension_service =
-      extensions::ExtensionSystem::Get(profile)->extension_service();
-  if (extension_service) {
-    std::set<std::string> extension_ids =
-        extension_service->process_map()->
-            GetExtensionsInProcess(child_process_id);
-    if (!extension_ids.empty())
-      // Since All the apps in a process share the same storage partition,
-      // we can pick any of them to retrieve the storage partition id.
-      extension =
-          extension_service->extensions()->GetByID(*(extension_ids.begin()));
-  }
-  return GetStoragePartitionIdForExtension(browser_context, extension);
-}
-
 std::string ChromeContentBrowserClient::GetStoragePartitionIdForSite(
     content::BrowserContext* browser_context,
     const GURL& site) {
-  const Extension* extension = NULL;
-  Profile* profile = Profile::FromBrowserContext(browser_context);
-  ExtensionService* extension_service =
-      extensions::ExtensionSystem::Get(profile)->extension_service();
-  if (extension_service) {
-    extension = extension_service->extensions()->
-        GetExtensionOrAppByURL(ExtensionURLInfo(site));
-  }
-
-  return GetStoragePartitionIdForExtension(browser_context, extension);
+  std::string app_id;
+  std::string partition_name;
+  bool in_memory = false;
+
+  // We need to get all the pieces that will go into the storage partition
+  // identifier first, before we compose it.
+  GetStoragePartitionConfigForSite(browser_context, site, &app_id,
+                                   &partition_name, &in_memory);
+
+  // If there is no app, we are in the default browser partition, so return
+  // an empty string.
+  if (app_id.empty())
+    return std::string();
+
+  // A non-empty storage partition id string is of the form
+  // "app_id:in_memory:partition_name", where each of the three parts is
+  // optional and the ':' separators are mandatory. Since "in-memory" is fixed
+  // string and the app_id cannot contain the separator, it is safe to parse
+  // the string based on the two separators.

[awong, 2012/11/05 23:48:37]

Question: How strong is our belief that |app_id|will not contain colons, and
what is the damage if one slips through?

Is it strong enough that we don't need a CHECK() for assertion?

[nasko, 2012/11/06 01:21:52]

The app id is the ascii representation of a public key hash. It should not have
any special characters in it.

+  std::string partition_id = base::StringPrintf("%s:%s:%s",
+                                                app_id.c_str(),
+                                                in_memory ? "in-memory" : "",
+                                                partition_name.c_str());
+
+  DCHECK(IsValidStoragePartitionId(browser_context,partition_id));
+  return partition_id;
 }
 
 bool ChromeContentBrowserClient::IsValidStoragePartitionId(
     content::BrowserContext* browser_context,
     const std::string& partition_id) {
-  // The default ID is empty which is always allowed.
+  // The default ID is empty and is always valid.
   if (partition_id.empty())
     return true;
 
-  // If it isn't empty, then it must belong to an extension of some sort. Parse
-  // out the extension ID and make sure it is still installed.
-  Profile* profile = Profile::FromBrowserContext(browser_context);
-  ExtensionService* extension_service =
+  // Now, parse the three parts of the partition ID, so we can verify them.

[Charlie Reis, 2012/11/06 00:17:18]

Yeah, let's chat about this in person tomorrow.  If we can avoid adding a custom
parser to the browser process, I'd sleep a little better.  :)

[nasko, 2012/11/07 00:33:57]

Done.

+  std::vector<std::string> tokens;
+  base::SplitString(partition_id, ':', &tokens);
+
+  if (tokens.size() < 3) {

[awong, 2012/11/05 23:48:37]

Should it be exactly 3?

[nasko, 2012/11/06 01:21:52]

Not if the partition name itself has ':'. Since we are not filtering that input
from the app, it can be "foo:bar:goog". This is the reason the partition name is
at the end of the string, since the other two strings are guaranteed to be
delimiter free.

+    NOTREACHED();
+    return false;
+  }
+
+  // Starting off with the app id, verify it exists.
+  if (!tokens[0].empty()) {
+    Profile* profile = Profile::FromBrowserContext(browser_context);
+    ExtensionService* extension_service =
       extensions::ExtensionSystem::Get(profile)->extension_service();
-  if (!extension_service) {
+
     // No extension service means no storage partitions in Chrome.
+    if (!extension_service)
+      return false;
+    if (extension_service->GetExtensionById(tokens[0], false) == NULL)
+      return false;
+  }
+
+  // The second token is either empty or the "in-memory" string.
+  if (!tokens[1].empty() && tokens[1] != "in-memory")
     return false;
+
+  return true;
+}
+
+void ChromeContentBrowserClient::GetStoragePartitionConfigForSite(
+    content::BrowserContext* browser_context,
+    const GURL& site,
+    std::string* app_id,
+    std::string* partition_name,
+    bool* in_memory) {
+  // For the webview tag, we create special guest processes, which host the
+  // tag content separately from the main application that embeds the tag.
+  // A webview tag can specify both the partition name and whether the storage
+  // for that partition should be persisted. Each tag gets a SiteInstance with
+  // a specially formatted URL, based on the application it is hosted by and
+  // the partition requested by it. The format for that URL is:
+  // guest://app_id/persist?partition_name
+  if (site.SchemeIs(chrome::kGuestScheme)) {
+    // Since guest URLs are only used for packaged apps, there must be an app
+    // id in the URL.
+    CHECK(site.has_host());
+    *app_id = site.host();
+    // Since persistence is optional, the path must either be empty or the
+    // literal string.
+    *in_memory = (site.path() != "/persist");
+    // The partition name is user supplied value, which we have encoded when the
+    // URL was created, so it needs to be decoded.
+    *partition_name = net::UnescapeURLComponent(site.query(),
+                                                net::UnescapeRule::NORMAL);
+    return;
+  }
+
+  const Extension* extension = NULL;
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  ExtensionService* extension_service =
+      extensions::ExtensionSystem::Get(profile)->extension_service();
+  if (extension_service) {
+    extension = extension_service->extensions()->
+        GetExtensionOrAppByURL(ExtensionURLInfo(site));
+    if (extension && extension->is_storage_isolated()) {
+      *app_id = extension->id();
+      *partition_name = std::string();

[awong, 2012/11/05 23:48:37]

partition_name->clear()?

[nasko, 2012/11/06 01:21:52]

Done.

+      *in_memory = false;
+      return;
+    }
   }
 
-  // See if we can find an extension. The |partition_id| is the extension ID so
-  // no parsing needed to be done.
-  return extension_service->GetExtensionById(partition_id, false) != NULL;
+  *app_id = std::string();

[awong, 2012/11/05 23:48:37]

->clear(), here and below?

[nasko, 2012/11/06 01:21:52]

Done.

+  *partition_name = std::string();
+  *in_memory = false;
 }
 
 content::WebContentsViewDelegate*
@@ -1608,10 +1668,16 @@ void ChromeContentBrowserClient::OverrideWebkitPrefs(
   chrome::ViewType view_type = chrome::GetViewType(web_contents);
   ExtensionService* service = profile->GetExtensionService();
   if (service) {
-    const Extension* extension = service->extensions()->GetByID(
-        rvh->GetSiteInstance()->GetSiteURL().host());
-    extension_webkit_preferences::SetPreferences(
-        extension, view_type, web_prefs);
+    const GURL& url = rvh->GetSiteInstance()->GetSiteURL();
+    const Extension* extension = service->extensions()->GetByID(url.host());
+    // Ensure that we are only granting extension preferences to URLs with
+    // the correct scheme. Without this check, guest:// schemes used by
+    // webview tags as well as hosts that happen to match the id of an
+    // installed extension would get the wrong preferences.
+    if (url.SchemeIs(chrome::kExtensionScheme)) {
+      extension_webkit_preferences::SetPreferences(
+          extension, view_type, web_prefs);
+    }
   }
 
   if (content::IsForceCompositingModeEnabled())
@@ -1868,23 +1934,4 @@ void ChromeContentBrowserClient::SetApplicationLocaleOnIOThread(
   io_thread_application_locale_ = locale;
 }
 
-std::string ChromeContentBrowserClient::GetStoragePartitionIdForExtension(
-    content::BrowserContext* browser_context, const Extension* extension) {
-  // In chrome, we use the extension ID as the partition ID. This works well
-  // because the extension ID fits the partition ID pattern and currently only
-  // apps can designate that storage should be isolated.
-  //
-  // If |extension| is NULL, then the default, empty string, partition id is
-  // used.
-  std::string partition_id;
-  if (extension && extension->is_storage_isolated()) {
-    partition_id = extension->id();
-  }
-
-  // Enforce that IsValidStoragePartitionId() implementation stays in sync.
-  DCHECK(IsValidStoragePartitionId(browser_context, partition_id));
-  return partition_id;
-}
-
-
 }  // namespace chrome