<browser>: Always read <browser>.src attribute from <object>.src.

content/browser/browser_plugin/browser_plugin_embedder.cc

Index: content/browser/browser_plugin/browser_plugin_embedder.cc
diff --git a/content/browser/browser_plugin/browser_plugin_embedder.cc b/content/browser/browser_plugin/browser_plugin_embedder.cc
index 091c00c90b0bb8c400514743bb4431de4d7e1599..ccdefe50015cfd4b9d8b305d09034b3fe04a6c0e 100644
--- a/content/browser/browser_plugin/browser_plugin_embedder.cc
+++ b/content/browser/browser_plugin/browser_plugin_embedder.cc
@@ -122,17 +122,15 @@ void BrowserPluginEmbedder::NavigateGuest(
   WebContentsImpl* guest_web_contents =
       static_cast<WebContentsImpl*>(guest->GetWebContents());
 
-  // We ignore loading empty urls in web_contents.
+  // We do not load empty urls in web_contents.
   // If a guest sets empty src attribute after it has navigated to some
-  // non-empty page, the action is considered no-op.
-  // TODO(lazyboy): The js shim for browser-plugin might need to reflect empty
-  // src ignoring in the shadow DOM element: http://crbug.com/149001.
-  if (!src.empty()) {
-    guest_web_contents->GetController().LoadURL(url,
-                                                Referrer(),
-                                                PAGE_TRANSITION_AUTO_SUBFRAME,
-                                                std::string());
-  }
+  // non-empty page, the action is considered no-op. This empty src navigation
+  // should never be sent to BrowserPluginEmbedder (browser process).
+  CHECK(!src.empty());

[Charlie Reis, 2012/10/15 18:57:28]

Hmm, we've got a few problems here, actually.  Keep in mind that renderer
processes aren't supposed to be trusted.  If they're exploited, they might send
any values back to the browser process.  So here, the renderer could kill the
browser process by sending an empty URL.

Worse, we're skipping the security checks we normally do about whether a URL is
allowed to be loaded.  (See uses of FilterURL in RenderViewHostImpl.)

Here's what I'd recommend: change this to a DCHECK but also keep the if, so that
we catch bugs in debug builds but don't crash in the wild.  Then put a
TODO(creis) comment to check the validity of the URL, referencing
http://crbug.com/139397.

[lazyboy, 2012/10/15 20:45:57]

Done.

+  guest_web_contents->GetController().LoadURL(url,
+                                              Referrer(),
+                                              PAGE_TRANSITION_AUTO_SUBFRAME,
+                                              std::string());
 
   // Resize the guest if the resize parameter was set from the renderer.
   ResizeGuest(render_view_host, instance_id, resize_params);