Restrict extensionview to chrome-extension://

extensions/browser/guest_view/extension_view/extension_view_guest.cc

Index: extensions/browser/guest_view/extension_view/extension_view_guest.cc
diff --git a/extensions/browser/guest_view/extension_view/extension_view_guest.cc b/extensions/browser/guest_view/extension_view/extension_view_guest.cc
index 952ec1331cfb1a9eacee3e86989253bd3f22326a..277ade1e8063da90b600c00da03034a248ec398d 100644
--- a/extensions/browser/guest_view/extension_view/extension_view_guest.cc
+++ b/extensions/browser/guest_view/extension_view/extension_view_guest.cc
@@ -5,6 +5,7 @@
 #include "extensions/browser/guest_view/extension_view/extension_view_guest.h"
 
 #include "base/metrics/user_metrics.h"
+#include "components/crx_file/id_util.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/common/result_codes.h"
 #include "extensions/browser/api/extensions_api_client.h"
@@ -40,10 +41,7 @@ extensions::GuestViewBase* ExtensionViewGuest::Create(
 
 void ExtensionViewGuest::NavigateGuest(const std::string& src,
                                        bool force_navigation) {
-  if (src.empty())
-    return;
-
-  GURL url(src);
+  GURL url(GetExtensionViewPage(src));

[Fady Samuel, 2015/02/16 19:26:28]

I would follow WebViewGuest's code to navigate to about:blank if this is a bad
src.

Also, I would plumb loadcommit up to update the src attribute.

[apacible, 2015/02/17 18:31:25]

Navigates to about:blank, and followed WebViewGuest's method of checking src.

Haven't plumbed loadcommit yet. Should I also plumb loadabort?

[Fady Samuel, 2015/02/17 18:48:29]

I would plumb loadcommit at least. loadcommit should probably update the src
attribute as it does in <webview>.

   if (!url.is_valid() && !force_navigation && (url == view_page_))
     return;
 
@@ -63,20 +61,42 @@ bool ExtensionViewGuest::CanRunInDetachedState() const {
 void ExtensionViewGuest::CreateWebContents(
     const base::DictionaryValue& create_params,
     const WebContentsCreatedCallback& callback) {
-  std::string str;
-  if (!create_params.GetString(extensionview::kAttributeSrc, &str)) {
+  // Gets the extension ID.
+  std::string extension_id;
+  create_params.GetString(extensionview::kAttributeExtension, &extension_id);
+
+  if (!crx_file::id_util::IdIsValid(extension_id)) {
     callback.Run(nullptr);
     return;
   }
 
-  GURL source(str);
-  if (!source.is_valid()) {
+  // Gets the extension URL.
+  GURL extension_url_from_id =

[Fady Samuel, 2015/02/16 19:26:28]

I would keep things as a GURL rather than as a string.

[apacible, 2015/02/17 18:31:25]

Changed extension_url_ to a GURL.

+      extensions::Extension::GetBaseURLFromExtensionId(extension_id);
+
+  if (!extension_url_from_id.is_valid()) {
+    callback.Run(nullptr);
+    return;
+  }
+
+  extension_url_ = extension_url_from_id.spec();
+
+  // Get the src to build URL to render.
+  std::string src;
+  if (!create_params.GetString(extensionview::kAttributeSrc, &src)) {
+    callback.Run(nullptr);
+    return;
+  }
+
+  view_page_ = GURL(GetExtensionViewPage(src));
+  if (!view_page_.is_valid()) {
     callback.Run(nullptr);
     return;
   }
 
   content::SiteInstance* view_site_instance =
-      content::SiteInstance::CreateForURL(browser_context(), source);
+      content::SiteInstance::CreateForURL(browser_context(),
+                                          extension_url_from_id);
 
   WebContents::CreateParams params(browser_context(), view_site_instance);
   params.guest_delegate = this;
@@ -129,4 +149,11 @@ void ExtensionViewGuest::ApplyAttributes(const base::DictionaryValue& params) {
   NavigateGuest(src, false /* force_navigation */);
 }
 
+std::string ExtensionViewGuest::GetExtensionViewPage(std::string src) {
+  if (src.empty())
+    return NULL;
+
+  return extension_url_ + src;

[Fady Samuel, 2015/02/16 19:26:28]

I would model this after WebViewGuest::ResolveURL. There's a bunch of validity
checks that are built into GURL that are not captured here.

[apacible, 2015/02/17 18:31:26]

Done.

+}
+
 }  // namespace extensions