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

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
===================================================================
--- content/browser/web_contents/web_contents_impl.cc	(revision 154307)
+++ content/browser/web_contents/web_contents_impl.cc	(working copy)
@@ -160,6 +160,7 @@
 using content::SiteInstance;
 using content::UserMetricsAction;
 using content::WebContents;
+using content::WebContentsDelegate;
 using content::WebContentsObserver;
 using content::WebUI;
 using content::WebUIController;
@@ -1029,17 +1030,6 @@
   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();
 }
@@ -1275,25 +1265,29 @@
     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);
+    }
   }
 }
 
@@ -1333,8 +1327,13 @@
                                         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,