Pass result of blockage across content API when new tab blocked.

content/browser/web_contents/web_contents_impl.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/web_contents/web_contents_impl.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
@@ skipping 142 matching lines @@
 using content::RenderWidgetHost;
 using content::RenderWidgetHostImpl;
 using content::RenderWidgetHostView;
 using content::RenderWidgetHostViewPort;
 using content::ResourceDispatcherHostImpl;
 using content::SSLStatus;
 using content::SessionStorageNamespace;
 using content::SiteInstance;
 using content::UserMetricsAction;
 using content::WebContents;
+using content::WebContentsDelegate;
 using content::WebContentsObserver;
 using content::WebUI;
 using content::WebUIController;
 using content::WebUIControllerFactory;
 using webkit_glue::WebPreferences;
 
 namespace {
 
 // Amount of time we wait between when a key event is received and the renderer
 // is queried for its state and pushed to the NavigationEntry.
@@ skipping 849 matching lines @@
   // We use our current SiteInstance since the cloned entry will use it anyway.
   // We pass |this| for the |base_web_contents| to size the view correctly, and
   // our own opener so that the cloned page can access it if it was before.
   WebContentsImpl* tc = CreateWithOpener(GetBrowserContext(),
                                          GetSiteInstance(), MSG_ROUTING_NONE,
                                          this, opener_);
   tc->GetController().CopyStateFrom(controller_);
   return tc;
 }
 
-void WebContentsImpl::AddNewContents(WebContents* new_contents,
-                                     WindowOpenDisposition disposition,
-                                     const gfx::Rect& initial_pos,
-                                     bool user_gesture) {
-  if (!delegate_)
-    return;
-
-  delegate_->AddNewContents(this, new_contents, disposition, initial_pos,
-                            user_gesture);
-}
-
 gfx::NativeView WebContentsImpl::GetContentNativeView() const {
   return view_->GetContentNativeView();
 }
 
 gfx::NativeView WebContentsImpl::GetNativeView() const {
   return view_->GetNativeView();
 }
 
 void WebContentsImpl::GetContainerBounds(gfx::Rect* out) const {
   view_->GetContainerBounds(out);
@@ skipping 215 matching lines @@
     // TODO(brettw): It seems bogus that we have to call this function on the
     // newly created object and give it one of its own member variables.
     new_view->CreateViewForWidget(new_contents->GetRenderViewHost());
 
     // Save the created window associated with the route so we can show it
     // later.
     DCHECK_NE(MSG_ROUTING_NONE, route_id);
     pending_contents_[route_id] = new_contents;
   }
 
+  bool not_blocked = true;
   if (delegate_) {
-    delegate_->WebContentsCreated(
+    not_blocked = delegate_->WebContentsCreated(
         this, params.opener_frame_id, params.target_url, new_contents);
   }
 
-  if (params.opener_suppressed) {
+  if (params.opener_suppressed && not_blocked) {
     // When the opener is suppressed, the original renderer cannot access the
     // new window.  As a result, we need to show and navigate the window here.
-    gfx::Rect initial_pos;
-    // TODO(cdn) Fix popup white-listing for links that open in a new process.
-    AddNewContents(
-        new_contents, params.user_gesture ? params.disposition : NEW_POPUP,
-        initial_pos, params.user_gesture);
-
-    content::OpenURLParams open_params(params.target_url, content::Referrer(),
-                                       CURRENT_TAB,
-                                       content::PAGE_TRANSITION_LINK,
-                                       true /* is_renderer_initiated */);
-    new_contents->OpenURL(open_params);
+    if (delegate_) {
+      gfx::Rect initial_pos;
+      not_blocked = delegate_->AddNewContents(

[jam, 2012/09/04 19:50:46]

it's confusing to read this block of code, since the delegate is asked twice
whether they want to block this or not? why isn't the first call sufficient?

[Tom Sepez, 2012/09/04 20:46:16]

You're right.  I think the first call need not return bool and that this is the
only place that checks for blocking. I'll make the changes

+          this, new_contents, params.disposition, initial_pos,
+          params.user_gesture);
+    }
+    if (not_blocked) {
+      content::OpenURLParams open_params(params.target_url,
+                                         content::Referrer(),
+                                         CURRENT_TAB,
+                                         content::PAGE_TRANSITION_LINK,
+                                         true /* is_renderer_initiated */);
+      new_contents->OpenURL(open_params);
+    }
   }
 }
 
 void WebContentsImpl::CreateNewWidget(int route_id,
                                       WebKit::WebPopupType popup_type) {
   CreateNewWidget(route_id, false, popup_type);
 }
 
 void WebContentsImpl::CreateNewFullscreenWidget(int route_id) {
   CreateNewWidget(route_id, true, WebKit::WebPopupTypeNone);
@@ skipping 19 matching lines @@
   // to allow it to survive the trip without being hosted.
   base::mac::NSObjectRetain(widget_view->GetNativeView());
 #endif
 }
 
 void WebContentsImpl::ShowCreatedWindow(int route_id,
                                         WindowOpenDisposition disposition,
                                         const gfx::Rect& initial_pos,
                                         bool user_gesture) {
   WebContentsImpl* contents = GetCreatedWindow(route_id);
-  if (contents)
-    AddNewContents(contents, disposition, initial_pos, user_gesture);
+  if (contents) {
+    WebContentsDelegate* delegate = GetDelegate();
+    if (delegate) {
+      delegate->AddNewContents(
+          this, contents, disposition, initial_pos, user_gesture);
+    }
+  }
 }
 
 void WebContentsImpl::ShowCreatedWidget(int route_id,
                                         const gfx::Rect& initial_pos) {
   ShowCreatedWidget(route_id, false, initial_pos);
 }
 
 void WebContentsImpl::ShowCreatedFullscreenWidget(int route_id) {
   ShowCreatedWidget(route_id, true, gfx::Rect());
 }
@@ skipping 1905 matching lines @@
       old_browser_plugin_host()->embedder_render_process_host();
   *embedder_container_id = old_browser_plugin_host()->instance_id();
   int embedder_process_id =
       embedder_render_process_host ? embedder_render_process_host->GetID() : -1;
   if (embedder_process_id != -1) {
     *embedder_channel_name =
         StringPrintf("%d.r%d", render_view_host->GetProcess()->GetID(),
                      embedder_process_id);
   }
 }