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

content/browser/browser_plugin/browser_plugin_embedder.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 "content/browser/browser_plugin/browser_plugin_embedder.h"
 
 #include <set>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ skipping 104 matching lines @@
     RenderViewHost* render_view_host,
     int instance_id,
     const std::string& src,
     const BrowserPluginHostMsg_ResizeGuest_Params& resize_params) {
   BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
   CHECK(guest);
   GURL url(src);
   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);
 }
 
 void BrowserPluginEmbedder::UpdateRectACK(int instance_id,
                                           int message_id,
                                           const gfx::Size& size) {
   BrowserPluginGuest* guest = GetGuestByInstanceID(instance_id);
   if (guest)
@@ skipping 158 matching lines @@
       bool visible = *Details<bool>(details).ptr();
       WebContentsVisibilityChanged(visible);
       break;
     }
     default:
       NOTREACHED() << "Unexpected notification type: " << type;
   }
 }
 
 }  // namespace content