Handle more cases for disposition link clicks index

brave · Aug 26, 2017 · ddbc5d4 · ddbc5d4
1 parent 0dcbb1e
commit ddbc5d4
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 12 deletions.
diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc
@@ -649,8 +649,10 @@ void WebContents::AddNewContents(content::WebContents* source,
   if (disposition != WindowOpenDisposition::NEW_WINDOW &&
       disposition != WindowOpenDisposition::NEW_POPUP) {
     auto tab_helper = extensions::TabHelper::FromWebContents(new_contents);
-    if (tab_helper->get_index() == TabStripModel::kNoTab) {
+    if (tab_helper &&
+        tab_helper->get_index() == TabStripModel::kNoTab) {
       ::Browser* browser = nullptr;
+      bool active = disposition == WindowOpenDisposition::NEW_FOREGROUND_TAB;
       if (tab_helper->window_id() != -1) {
         browser = tab_helper->browser();
       } else {
@@ -660,10 +662,11 @@ void WebContents::AddNewContents(content::WebContents* source,
         int index =
           browser->tab_strip_model()->order_controller()->
             DetermineInsertionIndex(ui::PAGE_TRANSITION_LINK,
-                                    tab_helper->is_active() ?
-                                      TabStripModel::ADD_ACTIVE :
-                                      TabStripModel::ADD_NONE);
+                                    active ?
+                                    TabStripModel::ADD_ACTIVE :
+                                    TabStripModel::ADD_NONE);
         tab_helper->SetTabIndex(index);
+        tab_helper->SetActive(active);
       }
     }
   }
@@ -2631,29 +2634,31 @@ void WebContents::OnTabCreated(const mate::Dictionary& options,
     tab_helper->Discard();
   }
 
-  TabStripModel *tab_strip = nullptr;
   int window_id = -1;
+  ::Browser *browser = nullptr;
   if (options.Get("windowId", &window_id) && window_id != -1) {
     auto api_window =
         mate::TrackableObject<Window>::FromWeakMapID(isolate(), window_id);
     if (api_window) {
-      // TODO(bridiver) - combine these two methods
+      browser = api_window->window()->browser();
       tab_helper->SetWindowId(window_id);
-      tab_helper->SetBrowser(api_window->window()->browser());
     }
   }
+  if (!browser) {
+    browser = owner_window()->browser();
+  }
+
+  tab_helper->SetOpener(opener_tab_id);
+  tab_helper->SetBrowser(browser);
 
   content::WebContents* source = nullptr;
   if (opener_tab_id != TabStripModel::kNoTab) {
     source = extensions::TabHelper::GetTabById(opener_tab_id);
-    tab_helper->SetOpener(opener_tab_id);
   }
 
   if (!source)
     source = web_contents();
 
-  tab_helper->SetTabIndex(index);
-
   bool was_blocked = false;
   AddNewContents(source,
                     tab,

diff --git a/atom/browser/extensions/tab_helper.cc b/atom/browser/extensions/tab_helper.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_order_controller.h"
 #include "components/sessions/core/session_id.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/navigation_entry.h"
@@ -414,15 +415,28 @@ void TabHelper::SetBrowser(Browser* browser) {
       auto tab_strip = browser->tab_strip_model();
     }
 
-    if (index_ == TabStripModel::kNoTab ||
-        index_ > browser_->tab_strip_model()->count()) {
+    // When there is an opener tab and the index is not currently valid,
+    // we don't want to overwrite the index with the last tab index because
+    // the index will be determined by the opener tab.
+    bool is_invalid_tab_index = index_ == TabStripModel::kNoTab ||
+      index_ > browser_->tab_strip_model()->count();
+    if (opener_tab_id_ == TabStripModel::kNoTab &&
+        is_invalid_tab_index) {
       index_ = browser_->tab_strip_model()->count();
+    } else if (is_invalid_tab_index) {
+      index_ =
+        browser_->tab_strip_model()->order_controller()->
+        DetermineInsertionIndex(ui::PAGE_TRANSITION_LINK,
+                                active_ ?
+                                TabStripModel::ADD_ACTIVE :
+                                TabStripModel::ADD_NONE);
     }
 
     int add_types = TabStripModel::ADD_NONE;
     add_types |= active_ ? TabStripModel::ADD_ACTIVE : 0;
     add_types |= opener_tab_id_ != TabStripModel::kNoTab ?
       TabStripModel::ADD_INHERIT_OPENER : 0;
+
     browser_->tab_strip_model()->InsertWebContentsAt(
         index_, web_contents(), add_types);
   } else {