fix external protocol handling for OOPIFs

chrome/browser/ui/cocoa/external_protocol_dialog.mm

 // 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.
 
 #import "chrome/browser/ui/cocoa/external_protocol_dialog.h"
 
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "chrome/browser/external_protocol/external_protocol_handler.h"
 #include "chrome/browser/shell_integration.h"
+#include "chrome/browser/tab_contents/tab_util.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
+#include "content/public/browser/web_contents.h"

[msw, 2016/12/19 20:26:55]

optional nit: remove this include, it's not explicitly needed (type fwd declared
by tab_util.h and not dereferenced here)

[davidsac (gone - try alexmos), 2016/12/19 22:23:45]

Done.

 #include "ui/base/l10n/l10n_util_mac.h"
 #include "ui/gfx/text_elider.h"
 
 ///////////////////////////////////////////////////////////////////////////////
 // ExternalProtocolHandler
 
 // static
 void ExternalProtocolHandler::RunExternalProtocolDialog(
     const GURL& url, int render_process_host_id, int routing_id,
     ui::PageTransition page_transition, bool has_user_gesture) {
@@ skipping 84 matching lines @@
     ExternalProtocolHandler::SetBlockState(url_.scheme(), blockState);
     ExternalProtocolHandler::RecordMetrics(true);
   } else {
     ExternalProtocolHandler::RecordMetrics(false);
   }
 
   if (blockState == ExternalProtocolHandler::DONT_BLOCK) {
     UMA_HISTOGRAM_LONG_TIMES("clickjacking.launch_url",
                              base::Time::Now() - creation_time_);
 
-    ExternalProtocolHandler::LaunchUrlWithoutSecurityCheck(
-        url_, render_process_host_id_, routing_id_);
+    content::WebContents* web_contents =
+        tab_util::GetWebContentsByID(render_process_host_id_, routing_id_);

[sky, 2016/12/15 20:44:28]

How do you know this returns non-null by the time you get here?

[davidsac (gone - try alexmos), 2016/12/15 22:40:23]

We don't.  The first action in
ExternalProtocolHandler::LaunchUrlWithoutSecurityCheck is to make sure that the
web_contents is not null, and return if it is.  Does this seem like it would
work?  Or do you think it's better to do the null checks at its call sites?

[msw, 2016/12/19 20:26:55]

I can't claim to speak for Scott, but that seems okay to me.

[davidsac (gone - try alexmos), 2016/12/19 22:23:45]

Acknowledged.

+
+    ExternalProtocolHandler::LaunchUrlWithoutSecurityCheck(url_, web_contents);
   }
 
   [self autorelease];
 }
 
 @end