Merge pull request #8 from brave/blocked-opener

correctly handle suppressed openers
brave · Jan 31, 2016 · e706fd1 · e706fd1
2 parents ef47e55 + de655e2
commit e706fd1
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 15 deletions.
diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
@@ -328,7 +328,7 @@ void WebContents::WebContentsCreated(content::WebContents* source_contents,
 
   CreateFrom(isolate(), new_contents)->delayed_load_url_ = true;
 
-  if (IsGuest() && opener_render_frame_id == MSG_ROUTING_NONE) {
+  if (IsGuest()) {
     content::NavigationController::LoadURLParams load_url_params(target_url);
     // http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-95229140
     load_url_params.referrer = content::Referrer(GetURL(),
@@ -340,9 +340,6 @@ void WebContents::WebContentsCreated(content::WebContents* source_contents,
     load_url_params.frame_name = frame_name;
     CreateFrom(isolate(), new_contents)->delayed_load_url_params_.reset(
       new content::NavigationController::LoadURLParams(load_url_params));
-  } else {
-    // we will never end up here, but someone else
-    // using normal electron windows would I think
   }
 }
 
@@ -355,6 +352,15 @@ void WebContents::AddNewContents(content::WebContents* source,
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());
 
+  // was_blocked is null unless the opener is suppressed
+  if (was_blocked) {
+    *was_blocked = true;
+  } else {
+    // we won't need these because the opener will handle it
+    CreateFrom(isolate(), new_contents)->
+      delayed_load_url_params_.reset(nullptr);
+  }
+
   // set webPreferences
   base::DictionaryValue* web_preferences =
     WebContentsPreferences::FromWebContents(new_contents)->web_preferences();
@@ -374,8 +380,7 @@ void WebContents::AddNewContents(content::WebContents* source,
                   "ATOM_SHELL_GUEST_VIEW_MANAGER_TAB_OPEN",
                   new_tab_event,
                   "about:blank",
-                  delayed_load_url_params_.get()
-                    ? new_contents->GetMainFrame()->GetFrameName() : "",
+                  "",
                   disposition,
                   *options);
     return;
@@ -403,8 +408,7 @@ void WebContents::AddNewContents(content::WebContents* source,
                   "ATOM_SHELL_GUEST_WINDOW_MANAGER_WINDOW_OPEN",
                   window_open_event,
                   "about:blank",
-                  delayed_load_url_params_.get()
-                    ? new_contents->GetMainFrame()->GetFrameName() : "",
+                  "",
                   *options);
     return;
   }

diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc
@@ -79,15 +79,21 @@ void WebFrame::RegisterElementResizeCallback(
 void WebFrame::AttachGuest(int id) {
   // This is a workaround for a strange issue on windows with background tabs
   // libchromiumcontent doesn't appear to be making the check for
-  // params.dispostion == NEW_BACKGROUND_TAB in WebContentsImpl
+  // params.disposition == NEW_BACKGROUND_TAB in WebContentsImpl
   // This results in the BrowserPluginGuest trying to access the native
-  // window before it's actually ready. This hack works around that
-  // by always delaying the access
-  content::BrowserPluginManager::Get()
-    ->GetBrowserPlugin(id)->updateVisibility(false);
+  // window before it's actually ready.
+  //
+  // It's also possible that the guest is being treated as
+  // visible because the "embedder", which is the same for all tabs
+  // in the window, is always visible.
+  //
+  // This hack works around the issue by always
+  // marking it as hidden while attaching
+  content::BrowserPluginManager::Get()->GetBrowserPlugin(id)->
+    updateVisibility(false);
   content::RenderFrame::FromWebFrame(web_frame_)->AttachGuest(id);
-  content::BrowserPluginManager::Get()
-    ->GetBrowserPlugin(id)->updateVisibility(true);
+  content::BrowserPluginManager::Get()->GetBrowserPlugin(id)->
+    updateVisibility(true);
 }
 
 void WebFrame::SetSpellCheckProvider(mate::Arguments* args,