Commit 94218d2c authored by Chris Hamilton's avatar Chris Hamilton Committed by Commit Bot

Make TabLoader responsible for loading all tabs.

Currently there's an inversion where active tabs in minimized windows don't get
loaded as they are created. This is an optimization in the occlusion tracking code
that doesn't calls ReloadIfNecessary unless the window hosting the active tab
actually becomes visible. Due to this inversion the TabLoader can start loading
background tabs in the minimized window before the active tab in the minimized
window is ever loaded.

Giving TabLoader responsibility to load all tabs fixes this. In the case of
active and visible tabs whose loads are initiated by the browser this is fine
because calling ReloadIfNecessary twice is effectively a nop, and the TabLoader
is already smart enough to track tabs that have started loading for external
reasons.

BUG=864725

Change-Id: I4b86a71470b2cb1a1a9106c4d4d92ee6b18c7284
Reviewed-on: https://chromium-review.googlesource.com/1141071Reviewed-by: default avatarSébastien Marchand <sebmarchand@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576071}
parent 8ae69efd
......@@ -657,10 +657,6 @@ class SessionRestoreImpl : public content::NotificationObserver {
tab.extension_app_id, is_selected_tab, tab.pinned, true,
last_active_time, session_storage_namespace.get(),
tab.user_agent_override, true /* from_session_restore */);
// Regression check: if the current tab |is_selected_tab|, it should load
// immediately, otherwise, tabs should not start loading right away. The
// focused tab will be loaded by Browser, and TabLoader will load the rest.
DCHECK(is_selected_tab || web_contents->GetController().NeedsReload());
// RestoreTab can return nullptr if |tab| doesn't have valid data.
if (!web_contents)
......
......@@ -207,7 +207,7 @@ void TabLoader::StartLoading(const std::vector<RestoredTab>& tabs) {
}
}
AddTab(restored_tab.contents(), restored_tab.is_active());
AddTab(restored_tab.contents());
}
StartTimerIfNeeded();
......@@ -347,7 +347,7 @@ size_t TabLoader::GetMaxNewTabLoads() const {
return tabs_to_load;
}
void TabLoader::AddTab(WebContents* contents, bool loading_initiated) {
void TabLoader::AddTab(WebContents* contents) {
DCHECK(reentry_depth_ > 0); // This can only be called internally.
// Handle tabs that have already started or finished loading.
......@@ -360,15 +360,7 @@ void TabLoader::AddTab(WebContents* contents, bool loading_initiated) {
return;
}
// Otherwise place it in one of the |tabs_load_initiated_| or
// |tabs_to_load_| containers.
if (loading_initiated) {
delegate_->NotifyTabLoadStarted();
++scheduled_to_load_count_;
tabs_load_initiated_.insert(contents);
} else {
tabs_to_load_.push_back(contents);
}
tabs_to_load_.push_back(contents);
}
void TabLoader::RemoveTab(WebContents* contents) {
......
......@@ -127,7 +127,7 @@ class TabLoader : public base::RefCounted<TabLoader>,
// Adds a tab that we are responsible for to one of the |tabs_*| containers.
// Can invalidate self-destroy and timer invariants.
void AddTab(content::WebContents* contents, bool loading_initiated);
void AddTab(content::WebContents* contents);
// Removes the tab from the set of tabs to load and list of tabs we're waiting
// to get a load from. Can invalidate self-destroy and timer invariants.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment