<webview>: Resolve relative paths as chrome-extension: URLs

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>
 
@@ skipping 805 matching lines @@
   CHECK(can_be_default || !partition_domain->empty());
 }
 
 content::WebContentsViewDelegate*
     ChromeContentBrowserClient::GetWebContentsViewDelegate(
         content::WebContents* web_contents) {
   return chrome::CreateWebContentsViewDelegate(web_contents);
 }
 
 void ChromeContentBrowserClient::GuestWebContentsCreated(
+    SiteInstance* guest_site_instance,

[Charlie Reis, 2013/10/30 20:42:20]

Is this equal to guest_web_contents->GetSiteInstance()?

If so, we don't need to pass it in explicitly, since we can get it from
guest_web_contents.

[Fady Samuel, 2013/10/31 17:25:17]

Yes, but GuestWebContentsCreated gets called prior to WebContentsImpl::Init and
so guest_web_contents->GetSiteInstance() is not yet available. This is necessary
because we need to install a GuestView prior to any kind of navigation so that
we don't miss events. 

     WebContents* guest_web_contents,
     WebContents* opener_web_contents,
     content::BrowserPluginGuestDelegate** guest_delegate,
     scoped_ptr<base::DictionaryValue> extra_params) {
+  if (!guest_site_instance) {
+    NOTREACHED();
+    return;
+  }
+  GURL guest_site_url = guest_site_instance->GetSiteURL();
+  const std::string& extension_id = guest_site_url.host();

[Charlie Reis, 2013/10/30 20:42:20]

I've lost track-- are we guaranteeing that the guest's SiteInstance has an
extension host, even in the WebUI and unprivileged (for zork@) cases?

[Fady Samuel, 2013/10/31 17:25:17]

For WebUI, this will be extracted from the iframe's URL in
BrowserPluginGuestManager::CreateGuest.

For unprivileged, we simply don't need an extension ID. Extension bindings will
not operate on BrowserPlugin. Neither WebViewGuest nor AdViewGuest will be
installed.

I expect a QuickOfficeGuest will be installed and it simply won't care about
extension IDs. We can add a check here to validate whether the host is a valid
extension ID and if not it can clear the string. I can do that now or in a
separate CL.

+
   if (opener_web_contents) {
     GuestView* guest = GuestView::FromWebContents(opener_web_contents);
     if (!guest) {
       NOTREACHED();
       return;
     }
 
     // Create a new GuestView of the same type as the opener.
     *guest_delegate =
-        GuestView::Create(guest_web_contents, guest->GetViewType());
+        GuestView::Create(guest_web_contents,
+                          extension_id,
+                          guest->GetViewType());
     return;
   }
 
   if (!extra_params) {
     NOTREACHED();
     return;
   }
   std::string api_type;
   extra_params->GetString(guestview::kParameterApi, &api_type);
 
+  if (api_type.empty())
+    return;
+
   *guest_delegate =
       GuestView::Create(guest_web_contents,
+                        extension_id,
                         GuestView::GetViewTypeFromString(api_type));
 }
 
 void ChromeContentBrowserClient::GuestWebContentsAttached(
     WebContents* guest_web_contents,
     WebContents* embedder_web_contents,
-    const GURL& embedder_frame_url,
     const base::DictionaryValue& extra_params) {
-  Profile* profile = Profile::FromBrowserContext(
-      embedder_web_contents->GetBrowserContext());
-  ExtensionService* service =
-      extensions::ExtensionSystem::Get(profile)->extension_service();
-  if (!service) {
-    NOTREACHED();
-    return;
-  }
 
-  // We usually require BrowserPlugins to be hosted by a storage isolated
-  // extension. We treat WebUI pages as a special case if they host the
-  // BrowserPlugin in a component extension iframe. In that case, we use the
-  // iframe's URL to determine the extension.

[Charlie Reis, 2013/10/30 20:42:20]

I haven't been able to figure out why it's safe to remove this.  Looks like your
CL gets the extension ID from the SiteInstance instead, but how does it get
there in the WebUI case?

[Fady Samuel, 2013/10/31 17:25:17]

See BrowserPluginGuestManager::CreateGuest. This is essentially code
duplication. We've already determined the guest_site_instance and made the
appropriate extension ID the host of the guest site instance. There's no reason
to determine the extension ID again here when we can simply extract it from the
guest site instance.

-  const GURL& embedder_site_url =
-      embedder_web_contents->GetSiteInstance()->GetSiteURL();
-  const Extension* extension = service->extensions()->GetExtensionOrAppByURL(
-      content::HasWebUIScheme(embedder_site_url) ?
-          embedder_frame_url : embedder_site_url);
-  if (!extension) {
+  GuestView* guest = GuestView::FromWebContents(guest_web_contents);
+  if (!guest) {
     // It's ok to return here, since we could be running a browser plugin
     // outside an extension, and don't need to attach a
     // BrowserPluginGuestDelegate in that case;
     // e.g. running with flag --enable-browser-plugin-for-all-view-types.
     return;
   }
-
-  GuestView* guest = GuestView::FromWebContents(guest_web_contents);
-  if (!guest) {
-    NOTREACHED();
-    return;
-  }
-  guest->Attach(embedder_web_contents, extension->id(), extra_params);
+  guest->Attach(embedder_web_contents, extra_params);
 }
 
 void ChromeContentBrowserClient::RenderProcessHostCreated(
     content::RenderProcessHost* host) {
   int id = host->GetID();
   Profile* profile = Profile::FromBrowserContext(host->GetBrowserContext());
   net::URLRequestContextGetter* context =
       profile->GetRequestContextForRenderProcess(id);
 
   host->AddFilter(new ChromeRenderMessageFilter(id, profile, context));
@@ skipping 1734 matching lines @@
   return IsExtensionOrSharedModuleWhitelisted(url, extension_set,
                                               allowed_file_handle_origins_) ||
          IsHostAllowedByCommandLine(url, extension_set,
                                     switches::kAllowNaClFileHandleAPI);
 #else
   return false;
 #endif
 }
 
 }  // namespace chrome