Service CreateNewWindow on the UI thread with a new mojo interface

chrome/browser/chrome_content_browser_client.cc

Index: chrome/browser/chrome_content_browser_client.cc
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 93dfff21e8bf4f23ed1a6c75efd39a2988821ad7..a85218ba56a52b08b261e00a6c3ab87f2a349187 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -779,20 +779,16 @@ void SetApplicationLocaleOnIOThread(const std::string& locale) {
   g_io_thread_application_locale.Get() = locale;
 }
 
-void HandleBlockedPopupOnUIThread(const BlockedWindowParams& params) {
-  RenderFrameHost* render_frame_host = RenderFrameHost::FromID(
-      params.render_process_id(), params.opener_render_frame_id());
-  if (!render_frame_host)
-    return;
-  WebContents* tab = WebContents::FromRenderFrameHost(render_frame_host);
+void HandleBlockedPopupOnUIThread(const BlockedWindowParams& params,
+                                  RenderFrameHost* opener,
+                                  WebContents* tab) {
+  DCHECK(tab);
+  DCHECK(opener);
   // The tab might already have navigated away.  We only need to do this check
   // for main frames, since the RenderFrameHost for a subframe opener will have
   // already been deleted if the main frame navigates away.

[ncarter (slow), 2017/04/20 21:30:36]

If you add the "frame_tree_node_->current_frame_host() != this" check to the top
of RenderFrameHostImpl::CreateNewWindow as I suggest in a later comment, then we
can remove this conditional block.

(Also: with oopifs, non-root pending delete hosts are possible, so this logic
had a hole that would have needed to be addressed anyway)

[Charlie Harrison, 2017/04/21 15:30:07]

Done.

-  if (!tab ||
-      (!render_frame_host->GetParent() &&
-       tab->GetMainFrame() != render_frame_host))
+  if (!opener->GetParent() && tab->GetMainFrame() != opener)
     return;
-
   prerender::PrerenderContents* prerender_contents =
       prerender::PrerenderContents::FromWebContents(tab);
   if (prerender_contents) {
@@ -807,22 +803,6 @@ void HandleBlockedPopupOnUIThread(const BlockedWindowParams& params) {
   popup_helper->AddBlockedPopup(params);
 }
 
-#if BUILDFLAG(ENABLE_PLUGINS)
-void HandleFlashDownloadActionOnUIThread(int render_process_id,
-                                         int render_frame_id,
-                                         const GURL& source_url) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  RenderFrameHost* render_frame_host =
-      RenderFrameHost::FromID(render_process_id, render_frame_id);
-  if (!render_frame_host)
-    return;
-  WebContents* web_contents =
-      WebContents::FromRenderFrameHost(render_frame_host);
-  FlashDownloadInterception::InterceptFlashDownloadNavigation(web_contents,
-                                                              source_url);
-}
-#endif  // BUILDFLAG(ENABLE_PLUGINS)
-
 // An implementation of the SSLCertReporter interface used by
 // SSLErrorHandler. Uses CertificateReportingService to send reports. The
 // service handles queueing and re-sending of failed reports. Each certificate
@@ -866,14 +846,6 @@ float GetDeviceScaleAdjustment() {
   return ratio * (kMaxFSM - kMinFSM) + kMinFSM;
 }
 
-void HandleSingleTabModeBlockOnUIThread(const BlockedWindowParams& params) {
-  WebContents* web_contents = tab_util::GetWebContentsByFrameID(
-      params.render_process_id(), params.opener_render_frame_id());
-  if (!web_contents)
-    return;
-
-  SingleTabModeTabHelper::FromWebContents(web_contents)->HandleOpenUrl(params);
-}
 #endif  // defined(OS_ANDROID)
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
@@ -999,6 +971,25 @@ WebContents* GetWebContents(int render_process_id, int render_frame_id) {
   return WebContents::FromRenderFrameHost(rfh);
 }
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+// Returns true if there is is an extension with the same origin as
+// |source_origin| in |opener_render_process_id| with
+// APIPermission::kBackground.
+bool SecurityOriginHasExtensionBackgroundPermission(
+    extensions::ProcessMap* process_map,
+    extensions::ExtensionRegistry* registry,
+    const GURL& source_origin,
+    int opener_render_process_id) {
+  // Note: includes web URLs that are part of an extension's web extent.
+  const Extension* extension =
+      registry->enabled_extensions().GetExtensionOrAppByURL(source_origin);
+  return extension &&
+         extension->permissions_data()->HasAPIPermission(
+             APIPermission::kBackground) &&
+         process_map->Contains(extension->id(), opener_render_process_id);
+}
+#endif
+
 }  // namespace
 
 ChromeContentBrowserClient::ChromeContentBrowserClient()
@@ -2458,8 +2449,7 @@ ChromeContentBrowserClient::GetPlatformNotificationService() {
 }
 
 bool ChromeContentBrowserClient::CanCreateWindow(
-    int opener_render_process_id,
-    int opener_render_frame_id,
+    RenderFrameHost* opener,
     const GURL& opener_url,
     const GURL& opener_top_level_frame_url,
     const GURL& source_origin,
@@ -2471,21 +2461,26 @@ bool ChromeContentBrowserClient::CanCreateWindow(
     const blink::mojom::WindowFeatures& features,
     bool user_gesture,
     bool opener_suppressed,
-    content::ResourceContext* context,
     bool* no_javascript_access) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(opener);
 
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(opener);
+  Profile* profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  DCHECK(profile);
   *no_javascript_access = false;
 
   // If the opener is trying to create a background window but doesn't have
   // the appropriate permission, fail the attempt.
   if (container_type == content::mojom::WindowContainerType::BACKGROUND) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-    ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
-    InfoMap* map = io_data->GetExtensionInfoMap();
-    if (!map->SecurityOriginHasAPIPermission(source_origin,
-                                             opener_render_process_id,
-                                             APIPermission::kBackground)) {
+    auto* process_map = extensions::ProcessMap::Get(profile);
+    auto* registry = extensions::ExtensionRegistry::Get(profile);
+    if (!SecurityOriginHasExtensionBackgroundPermission(
+            process_map, registry, source_origin,
+            opener->GetProcess()->GetID())) {
       return false;
     }
 
@@ -2496,7 +2491,7 @@ bool ChromeContentBrowserClient::CanCreateWindow(
     // already. We must use the full URL to find hosted apps, though, and not
     // just the origin.
     const Extension* extension =
-        map->extensions().GetExtensionOrAppByURL(opener_url);
+        registry->enabled_extensions().GetExtensionOrAppByURL(opener_url);
     if (extension && !extensions::BackgroundInfo::AllowJSAccess(extension))

[ncarter (slow), 2017/04/20 21:30:36]

Hah. It would appear that an extension can sidestep this restriction (and get JS
access) by adding a same-origin, different-scheme iframe; via about:blank,
blob:, or filesystem: ...

-- really, we ought to use the same extension here as the one looked up in
SecurityOriginHasExtensionBackgroundPermission. We can fix this now (which adds
risk to an already huge CL), or file a bug.

[Charlie Harrison, 2017/04/21 15:30:07]

Can you explain this a bit more? I'm happy to file a bug for this.

[ncarter (slow), 2017/04/21 20:42:13]

I filed my thoughts as crbug.com/714251

       *no_javascript_access = true;
 #endif
@@ -2506,18 +2501,16 @@ bool ChromeContentBrowserClient::CanCreateWindow(
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   if (extensions::WebViewRendererState::GetInstance()->IsGuest(
-          opener_render_process_id)) {
+          opener->GetProcess()->GetID())) {
     return true;
   }
 
   if (target_url.SchemeIs(extensions::kExtensionScheme)) {
-    // Intentionally duplicating |io_data| and |map| code from above because we
-    // want to reduce calls to retrieve them as this function is a SYNC IPC
-    // handler.
-    ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
-    InfoMap* map = io_data->GetExtensionInfoMap();
+    // Intentionally duplicating |registry| code from above because we want to
+    // reduce calls to retrieve them as this function is a SYNC IPC handler.
+    auto* registry = extensions::ExtensionRegistry::Get(profile);
     const Extension* extension =
-        map->extensions().GetExtensionOrAppByURL(target_url);
+        registry->enabled_extensions().GetExtensionOrAppByURL(target_url);
     if (extension && extension->is_platform_app()) {
       UMA_HISTOGRAM_ENUMERATION(
           "Extensions.AppLoadedInTab",
@@ -2531,47 +2524,41 @@ bool ChromeContentBrowserClient::CanCreateWindow(
 #endif
 
   HostContentSettingsMap* content_settings =
-      ProfileIOData::FromResourceContext(context)->GetHostContentSettingsMap();
+      HostContentSettingsMapFactory::GetForProfile(profile);
 
 #if BUILDFLAG(ENABLE_PLUGINS)
   if (FlashDownloadInterception::ShouldStopFlashDownloadAction(
           content_settings, opener_top_level_frame_url, target_url,
           user_gesture)) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        base::BindOnce(&HandleFlashDownloadActionOnUIThread,
-                       opener_render_process_id, opener_render_frame_id,
-                       opener_top_level_frame_url));
+    FlashDownloadInterception::InterceptFlashDownloadNavigation(
+        web_contents, opener_top_level_frame_url);
     return false;
   }
 #endif
 
-  BlockedWindowParams blocked_params(
-      target_url, referrer, frame_name, disposition, features, user_gesture,
-      opener_suppressed, opener_render_process_id, opener_render_frame_id);
-
   if (!user_gesture &&
       !base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisablePopupBlocking)) {
-    if (content_settings->GetContentSetting(opener_top_level_frame_url,
-                                            opener_top_level_frame_url,
-                                            CONTENT_SETTINGS_TYPE_POPUPS,
-                                            std::string()) !=
-        CONTENT_SETTING_ALLOW) {
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          base::BindOnce(&HandleBlockedPopupOnUIThread, blocked_params));
+    if (content_settings->GetContentSetting(
+            opener_top_level_frame_url, opener_top_level_frame_url,
+            CONTENT_SETTINGS_TYPE_POPUPS,
+            std::string()) != CONTENT_SETTING_ALLOW) {
+      BlockedWindowParams blocked_params(target_url, referrer, frame_name,
+                                         disposition, features, user_gesture,
+                                         opener_suppressed);
+      HandleBlockedPopupOnUIThread(blocked_params, opener, web_contents);
       return false;
     }
   }
 
 #if defined(OS_ANDROID)
-  if (SingleTabModeTabHelper::IsRegistered(opener_render_process_id,
-                                           opener_render_frame_id)) {
-    BrowserThread::PostTask(BrowserThread::UI,
-                            FROM_HERE,
-                            base::Bind(&HandleSingleTabModeBlockOnUIThread,
-                                       blocked_params));
+  auto* single_tab_mode_helper =
+      SingleTabModeTabHelper::FromWebContents(web_contents);
+  if (single_tab_mode_helper->block_all_new_windows()) {
+    BlockedWindowParams blocked_params(target_url, referrer, frame_name,
+                                       disposition, features, user_gesture,
+                                       opener_suppressed);
+    single_tab_mode_helper->HandleOpenUrl(blocked_params);
     return false;
   }
 #endif