diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 0187208241..3c55efb11d 100644 --- 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::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 index 26e4aed28b..35d70243c6 100644 --- 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 {