Webview tag creation should be using storage partitions.

chrome/browser/chrome_content_browser_client.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/browser/chrome_content_browser_client.h"
 
 #include <set>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/path_service.h"
 #include "base/string_tokenizer.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/breakpad_mac.h"
 #include "chrome/browser/browser_about_handler.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browsing_data/browsing_data_helper.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
 #include "chrome/browser/character_encoding.h"
 #include "chrome/browser/chrome_benchmarking_message_filter.h"
 #include "chrome/browser/chrome_quota_permission_context.h"
 #include "chrome/browser/content_settings/content_settings_utils.h"
@@ skipping 68 matching lines @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_view.h"
 #include "content/public/common/child_process_host.h"
 #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"
 #include "ppapi/host/ppapi_host.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "webkit/glue/webpreferences.h"
 #include "webkit/plugins/plugin_switches.h"
 
 #if defined(OS_WIN)
@@ skipping 348 matching lines @@
   return main_parts;
 }
 
 content::WebContentsView*
     ChromeContentBrowserClient::OverrideCreateWebContentsView(
         WebContents* web_contents,
         content::RenderViewHostDelegateView** render_view_host_delegate_view) {
   return NULL;
 }
 
-std::string ChromeContentBrowserClient::GetStoragePartitionIdForChildProcess(
+std::string ChromeContentBrowserClient::GetStoragePartitionIdForSite(
+      content::BrowserContext* browser_context,
+      const GURL& site) {
+  std::string app_id;
+  std::string partition_name;
+  bool in_memory;

[Charlie Reis, 2012/11/05 18:15:37]

Should probably initialize this, in case a bug later crops up in
GetStoragePartitionConfigForSite (e.g., return without setting it).

[nasko, 2012/11/05 19:20:26]

Done.

+
+  // 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

[Charlie Reis, 2012/11/05 18:15:37]

I'm not entirely clear on why we need this format.  I thought we were using site
URLs because they already know how to parse what we need.  (Seems safer to avoid
adding a custom parser to the browser process if we can.)

Wouldn't the site URL have enough information?

[nasko, 2012/11/05 19:20:26]

I've had the same discussion with Albert. He feels strongly against using the
URL as identifier, as different parts of the code may start depending on the
fact that it is URL of specific format and start taking parts of it. This is the
reason for this separate way to identify them.

[awong, 2012/11/05 20:11:18]

My objection was that GURL had too much validation logic. Could be convinced
otherwise though.

+  // 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

[Charlie Reis, 2012/11/05 18:15:37]

What are the restrictions on the partition_name?  Can it contain the separator? 
(I suppose you could still safely parse it, but it's not as obvious by looking
at it.)

[nasko, 2012/11/05 19:20:26]

I do have a bug here, the user supplied partition value doesn't have any
limitations at this point in time. I have to account for that in the parsing and
not expect exactly 2 separators.

+  // the string based on the two separators.
+  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,
-    int child_process_id) {
-  const Extension* extension = NULL;
+    const std::string& partition_id) {
+  // The default ID is empty and is always valid.
+  if (partition_id.empty())
+    return true;
+
   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()));
+    extensions::ExtensionSystem::Get(profile)->extension_service();
+
+  // Now, parse the three parts of the partition ID, so we can verify them.
+  // Set the tokenizer to return delimiters, otherwise we won't get the correct
+  // count of delimiters and will fail validation.
+  int token = 0;
+  StringTokenizer t(partition_id, ":");
+  t.set_options(StringTokenizer::RETURN_DELIMS);
+
+  while (t.GetNext()) {
+    if (t.token_is_delim()) {
+      token++;
+      continue;
+    }
+    switch (token) {
+      // Starting off with the app id, verify it exists.
+      case 0:
+        if (!t.token().empty())  {
+          // No extension service means no storage partitions in Chrome.
+          if (!extension_service)
+            return false;
+          if (extension_service->GetExtensionById(t.token(), false) == NULL)
+            return false;
+        }
+        break;
+      // The second token is either empty or the "in-memory" string.
+      case 1:
+        if (!t.token().empty() && t.token() != "in-memory")
+          return false;
+        break;
+      // We don't verify the partition_name, as it is user supplied and there is
+      // no format constraints to it.
+      case 2:
+        break;
+      // We only expect three parts in the partition_id string, fail otherwise.
+      default:
+        NOTREACHED();
+        return false;
+    }
   }
-  return GetStoragePartitionIdForExtension(browser_context, extension);
+
+  // If there weren't three tokens, even if empty, then it is not a valid
+  // partition id.
+  return (token == 2);
 }
 
-std::string ChromeContentBrowserClient::GetStoragePartitionIdForSite(
+void ChromeContentBrowserClient::GetStoragePartitionConfigForSite(
     content::BrowserContext* browser_context,
-    const GURL& site) {
+    const GURL& site,
+    std::string* app_id,
+    std::string* partition_name,
+    bool* in_memory) {
+  if (site.SchemeIs(chrome::kGuestScheme)) {

[Charlie Reis, 2012/11/05 18:15:37]

Please add some comments for each of these blocks, plus an example guest URL to
show what you're pulling out of it.

[nasko, 2012/11/05 19:20:26]

Done.

+    CHECK(site.has_host());
+    *app_id = site.host();
+    *partition_name = net::UnescapeURLComponent(site.query(),
+                                                net::UnescapeRule::NORMAL);
+    *in_memory = ((site.path() == "/persist") ? false : true);
+    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();
+      *in_memory = false;
+      return;
+    }
   }
 
-  return GetStoragePartitionIdForExtension(browser_context, extension);
-}
-
-bool ChromeContentBrowserClient::IsValidStoragePartitionId(
-    content::BrowserContext* browser_context,
-    const std::string& partition_id) {
-  // The default ID is empty which is always allowed.
-  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 =
-      extensions::ExtensionSystem::Get(profile)->extension_service();
-  if (!extension_service) {
-    // No extension service means no storage partitions in Chrome.
-    return false;
-  }
-
-  // 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();
+  *partition_name = std::string();
+  *in_memory = false;
 }
 
 content::WebContentsViewDelegate*
     ChromeContentBrowserClient::GetWebContentsViewDelegate(
         content::WebContents* web_contents) {
   return chrome::CreateWebContentsViewDelegate(web_contents);
 }
 
 void ChromeContentBrowserClient::RenderViewHostCreated(
     RenderViewHost* render_view_host) {
@@ skipping 1065 matching lines @@
   if (web_prefs->default_encoding.empty()) {
     prefs->ClearPref(prefs::kDefaultCharset);
     web_prefs->default_encoding = prefs->GetString(prefs::kDefaultCharset);
   }
   DCHECK(!web_prefs->default_encoding.empty());
 
   WebContents* web_contents = WebContents::FromRenderViewHost(rvh);
   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())
     web_prefs->force_compositing_mode = true;
 
   if (view_type == chrome::VIEW_TYPE_NOTIFICATION) {
     web_prefs->allow_scripts_to_close_windows = true;
   } else if (view_type == chrome::VIEW_TYPE_BACKGROUND_CONTENTS) {
     // Disable all kinds of acceleration for background pages.
     // See http://crbug.com/96005 and http://crbug.com/96006
@@ skipping 236 matching lines @@
               base::Unretained(this), locale)))
     io_thread_application_locale_ = locale;
 }
 
 void ChromeContentBrowserClient::SetApplicationLocaleOnIOThread(
     const std::string& locale) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   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