Make ContentSettingsObserver security checks work with OOPIF.

chrome/renderer/content_settings_observer.cc

Index: chrome/renderer/content_settings_observer.cc
diff --git a/chrome/renderer/content_settings_observer.cc b/chrome/renderer/content_settings_observer.cc
index 0f0eff8a6b54100522a5561cbe0947319b2b6b0c..323957895b1e5c51f476230bcee3a307cb5f40f1 100644
--- a/chrome/renderer/content_settings_observer.cc
+++ b/chrome/renderer/content_settings_observer.cc
@@ -115,9 +115,12 @@ static bool IsHostInDomain(const std::string& host, const std::string& domain) {
 }
 
 GURL GetOriginOrURL(const WebFrame* frame) {
-  WebString top_origin = frame->top()->document().securityOrigin().toString();
+  WebString top_origin = frame->top()->securityOrigin().toString();
   // The the |top_origin| is unique ("null") e.g., for file:// URLs. Use the

[Charlie Reis, 2014/12/12 18:02:45]

nit: Might as well fix "The the"

[alexmos, 2014/12/13 00:58:12]

Done.

   // document URL as the primary URL in those cases.
+  // TODO(alexmos): This is broken for --site-per-process, since top() can be a
+  // WebRemoteFrame which does not have a document(), and the WebRemoteFrame's
+  // URL is not currently replicated.

[Charlie Reis, 2014/12/12 18:02:45]

This makes me sad.  I'd like to argue against replicating the full URL if at all
possible.

Can we take a closer look at how this is used to see if they really need the URL
for unique origins?  Maybe there's a sane default behavior for unique origins
that doesn't require the URL.

[alexmos, 2014/12/13 00:58:12]

Hmm.  Similarly to what lazyboy@ found in
https://codereview.chromium.org/791763003/, when the top frame is a file:// URL,
it looks like it's possible for these rules to match patterns based on path in
the URL, though there's a TODO in
https://code.google.com/p/chromium/codesearch#chromium/src/components/content...
to stop supporting path-based matching in ContentSettingsPattern::Matches.  

There's a separate issue in that currently origin replication can't tell apart
file:// origins (with enforceFilePathSeparation set to true, which is the
default for chrome) and unique origins, since they're both serialized as "null".
 I will need to fix that at some point, and then just knowing that the top frame
came from file:// may be enough here, provided the TODO to remove path matching
gets done.

I also dug around and saw this related comment thread, which seems to suggest
this was intended for file:// URLs:
https://codereview.chromium.org/7831075/diff/84016/chrome/renderer/content_se....
It would still be good to know which content settings rules could actually apply
when the top origin is "null", and whether we'd break anything if we don't
support that.  Anyone we could ask?

[Charlie Reis, 2014/12/15 20:25:35]

From the TODOs and discussion, it looks like bauerb@, jochen@, and markusheintz@
are good candidates.

   if (top_origin == "null")
     return frame->top()->document().url();
   return GURL(top_origin);
@@ -269,23 +272,23 @@ bool ContentSettingsObserver::allowDatabase(const WebString& name,
                                             const WebString& display_name,
                                             unsigned long estimated_size) {
   WebFrame* frame = render_frame()->GetWebFrame();
-  if (frame->document().securityOrigin().isUnique() ||
-      frame->top()->document().securityOrigin().isUnique())
+  if (frame->securityOrigin().isUnique() ||
+      frame->top()->securityOrigin().isUnique())
     return false;
 
   bool result = false;
   Send(new ChromeViewHostMsg_AllowDatabase(
-      routing_id(), GURL(frame->document().securityOrigin().toString()),
-      GURL(frame->top()->document().securityOrigin().toString()),
-      name, display_name, &result));
+      routing_id(), GURL(frame->securityOrigin().toString()),
+      GURL(frame->top()->securityOrigin().toString()), name, display_name,
+      &result));
   return result;
 }
 
 void ContentSettingsObserver::requestFileSystemAccessAsync(
     const WebPermissionCallbacks& callbacks) {
   WebFrame* frame = render_frame()->GetWebFrame();
-  if (frame->document().securityOrigin().isUnique() ||
-      frame->top()->document().securityOrigin().isUnique()) {
+  if (frame->securityOrigin().isUnique() ||
+      frame->top()->securityOrigin().isUnique()) {
     WebPermissionCallbacks permissionCallbacks(callbacks);
     permissionCallbacks.doDeny();
     return;
@@ -299,10 +302,9 @@ void ContentSettingsObserver::requestFileSystemAccessAsync(
   DCHECK(insert_result.second);
 
   Send(new ChromeViewHostMsg_RequestFileSystemAccessAsync(
-      routing_id(),
-      current_request_id_,
-      GURL(frame->document().securityOrigin().toString()),
-      GURL(frame->top()->document().securityOrigin().toString())));
+      routing_id(), current_request_id_,
+      GURL(frame->securityOrigin().toString()),
+      GURL(frame->top()->securityOrigin().toString())));
 }
 
 bool ContentSettingsObserver::allowImage(bool enabled_per_settings,
@@ -331,15 +333,14 @@ bool ContentSettingsObserver::allowImage(bool enabled_per_settings,
 bool ContentSettingsObserver::allowIndexedDB(const WebString& name,
                                              const WebSecurityOrigin& origin) {
   WebFrame* frame = render_frame()->GetWebFrame();
-  if (frame->document().securityOrigin().isUnique() ||
-      frame->top()->document().securityOrigin().isUnique())
+  if (frame->securityOrigin().isUnique() ||
+      frame->top()->securityOrigin().isUnique())
     return false;
 
   bool result = false;
   Send(new ChromeViewHostMsg_AllowIndexedDB(
-      routing_id(), GURL(frame->document().securityOrigin().toString()),
-      GURL(frame->top()->document().securityOrigin().toString()),
-      name, &result));
+      routing_id(), GURL(frame->securityOrigin().toString()),
+      GURL(frame->top()->securityOrigin().toString()), name, &result));
   return result;
 }
 
@@ -397,8 +398,8 @@ bool ContentSettingsObserver::allowScriptFromSource(
 
 bool ContentSettingsObserver::allowStorage(bool local) {
   WebFrame* frame = render_frame()->GetWebFrame();
-  if (frame->document().securityOrigin().isUnique() ||
-      frame->top()->document().securityOrigin().isUnique())
+  if (frame->securityOrigin().isUnique() ||
+      frame->top()->securityOrigin().isUnique())
     return false;
   bool result = false;
 
@@ -410,9 +411,8 @@ bool ContentSettingsObserver::allowStorage(bool local) {
     return permissions->second;
 
   Send(new ChromeViewHostMsg_AllowDOMStorage(
-      routing_id(), GURL(frame->document().securityOrigin().toString()),
-      GURL(frame->top()->document().securityOrigin().toString()),
-      local, &result));
+      routing_id(), GURL(frame->securityOrigin().toString()),
+      GURL(frame->top()->securityOrigin().toString()), local, &result));
   cached_storage_permissions_[key] = result;
   return result;
 }