Changes session restore callback to always happen when tabs are created

chrome/browser/sessions/session_restore.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sessions/session_restore.h"
 
 #include <algorithm>
 #include <list>
 #include <set>
 #include <string>
@@ skipping 93 matching lines @@
   void TabIsLoading(NavigationController* controller);
 
   // Invokes |LoadNextTab| to load a tab.
   //
   // This must be invoked once to start loading.
   void StartLoading();
 
   // TabLoaderCallback:
   void SetTabLoadingEnabled(bool enable_tab_loading) override;
 
-  void set_on_session_restored_callbacks(
-      SessionRestore::CallbackList* callbacks) {
-    on_session_restored_callbacks_ = callbacks;
-  }
-
  private:
   friend class base::RefCounted<TabLoader>;
 
   typedef std::set<NavigationController*> TabsLoading;
   typedef std::list<NavigationController*> TabsToLoad;
   typedef std::set<RenderWidgetHost*> RenderWidgetHostSet;
 
   explicit TabLoader(base::TimeTicks restore_started);
   ~TabLoader() override;
 
@@ skipping 74 matching lines @@
   int tab_count_;
 
   base::OneShotTimer<TabLoader> force_load_timer_;
 
   // The time the restore process started.
   base::TimeTicks restore_started_;
 
   // Max number of tabs that were loaded in parallel (for metrics).
   size_t max_parallel_tab_loads_;
 
-  // Callback list for sending a session restore notification.
-  SessionRestore::CallbackList* on_session_restored_callbacks_;
-
   // For keeping TabLoader alive while it's loading even if no
   // SessionRestoreImpls reference it.
   scoped_refptr<TabLoader> this_retainer_;
 
   DISALLOW_COPY_AND_ASSIGN(TabLoader);
 };
 
 // static
 TabLoader* TabLoader::GetTabLoader(base::TimeTicks restore_started) {
   if (!shared_tab_loader)
@@ skipping 50 matching lines @@
     return;
   loading_enabled_ = enable_tab_loading;
   if (loading_enabled_)
     LoadNextTab();
   else
     force_load_timer_.Stop();
 }
 
 TabLoader::TabLoader(base::TimeTicks restore_started)
     : memory_pressure_listener_(
-        base::Bind(&TabLoader::OnMemoryPressure, base::Unretained(this))),
+          base::Bind(&TabLoader::OnMemoryPressure, base::Unretained(this))),
       force_load_delay_multiplier_(1),
       loading_enabled_(true),
       got_first_foreground_load_(false),
       got_first_paint_(false),
       tab_count_(0),
       restore_started_(restore_started),
-      max_parallel_tab_loads_(0),
-      on_session_restored_callbacks_(nullptr) {
+      max_parallel_tab_loads_(0) {
 }
 
 TabLoader::~TabLoader() {
   DCHECK((got_first_paint_ || render_widget_hosts_to_paint_.empty()) &&
           tabs_loading_.empty() && tabs_to_load_.empty());
   shared_tab_loader = NULL;
 }
 
 void TabLoader::LoadNextTab() {
   // LoadNextTab should only get called after we have started the tab
@@ skipping 19 matching lines @@
         // NOTE: We need to do this here rather than when the tab is added to
         // the Browser as at that time not everything has been created, so that
         // the call would do nothing.
         contents->WasHidden();
       }
     }
   }
 
   if (!tabs_to_load_.empty())
     StartTimer();
-
-  // When the session restore is done synchronously, notification is sent from
-  // SessionRestoreImpl::Restore .
-  if (tabs_to_load_.empty() && !SessionRestore::IsRestoringSynchronously()) {
-    on_session_restored_callbacks_->Notify(tab_count_);
-  }
 }
 
 void TabLoader::StartTimer() {
   force_load_timer_.Stop();
   force_load_timer_.Start(FROM_HERE,
                           delegate_->GetTimeoutBeforeLoadingNextTab() *
                               force_load_delay_multiplier_,
                           this, &TabLoader::ForceLoadTimerFired);
 }
 
@@ skipping 303 matching lines @@
 
     if (synchronous_) {
       {
         base::MessageLoop::ScopedNestableTaskAllower allow(
             base::MessageLoop::current());
         base::RunLoop loop;
         quit_closure_for_sync_restore_ = loop.QuitClosure();
         loop.Run();
         quit_closure_for_sync_restore_ = base::Closure();
       }
-      // Count the total number of tabs in |windows_|.
-      int total_num_tabs = 0;
-      for (int i = 0; i < static_cast<int>(windows_.size()); ++i)
-        total_num_tabs += windows_[i]->tabs.size();
-
-      Browser* browser = ProcessSessionWindows(&windows_, active_window_id_);
-      on_session_restored_callbacks_->Notify(total_num_tabs);
+      Browser* browser =
+          ProcessSessionWindowsAndNotify(&windows_, active_window_id_);
       delete this;
       return browser;
     }
 
     if (browser_) {
       registrar_.Add(this, chrome::NOTIFICATION_BROWSER_CLOSED,
                      content::Source<Browser>(browser_));
     }
 
     return browser_;
   }
 
   // Restore window(s) from a foreign session. Returns newly created Browsers.
   std::vector<Browser*> RestoreForeignSession(
       std::vector<const sessions::SessionWindow*>::const_iterator begin,
       std::vector<const sessions::SessionWindow*>::const_iterator end) {
     StartTabCreation();
     std::vector<Browser*> browsers;
+    std::vector<WebContents*> created_contents;
     // Create a browser instance to put the restored tabs in.
     for (std::vector<const sessions::SessionWindow*>::const_iterator i = begin;
          i != end; ++i) {
       Browser* browser = CreateRestoredBrowser(
           BrowserTypeForWindowType((*i)->type),
           (*i)->bounds,
           (*i)->show_state,
           (*i)->app_name);
       browsers.push_back(browser);
 
       // Restore and show the browser.
       const int initial_tab_count = 0;
       int selected_tab_index = std::max(
           0,
           std::min((*i)->selected_tab_index,
                    static_cast<int>((*i)->tabs.size()) - 1));
       RestoreTabsToBrowser(*(*i), browser, initial_tab_count,
-                           selected_tab_index);
+                           selected_tab_index, &created_contents);
       NotifySessionServiceOfRestoredTabs(browser, initial_tab_count);
     }
 
-    // Always create in a new window
+    // Always create in a new window.
     FinishedTabCreation(true, true);
+
+    on_session_restored_callbacks_->Notify(
+        static_cast<int>(created_contents.size()));
+
     return browsers;
   }
 
   // Restore a single tab from a foreign session.
   // Opens in the tab in the last active browser, unless disposition is
   // NEW_WINDOW, in which case the tab will be opened in a new browser. Returns
   // the WebContents of the restored tab.
   WebContents* RestoreForeignTab(const sessions::SessionTab& tab,
                                  WindowOpenDisposition disposition) {
     DCHECK(!tab.navigations.empty());
@@ skipping 42 matching lines @@
     if (use_new_window) {
       browser->tab_strip_model()->ActivateTabAt(0, true);
       browser->window()->Show();
     }
     NotifySessionServiceOfRestoredTabs(browser,
                                        browser->tab_strip_model()->count());
 
     // Since FinishedTabCreation() is not called here, |this| will leak if we
     // are not in sychronous mode.
     DCHECK(synchronous_);
+
+    on_session_restored_callbacks_->Notify(1);
+
     return web_contents;
   }
 
   ~SessionRestoreImpl() override {
     STLDeleteElements(&windows_);
 
     active_session_restorers->erase(this);
     if (active_session_restorers->empty()) {
       delete active_session_restorers;
       active_session_restorers = NULL;
@@ skipping 15 matching lines @@
         break;
     }
   }
 
   Profile* profile() { return profile_; }
 
  private:
   // Invoked when beginning to create new tabs. Resets the |tab_loader_|.
   void StartTabCreation() {
     tab_loader_ = TabLoader::GetTabLoader(restore_started_);
-    tab_loader_->set_on_session_restored_callbacks(
-        on_session_restored_callbacks_);
   }
 
   // Invoked when done with creating all the tabs/browsers.
   //
   // |created_tabbed_browser| indicates whether a tabbed browser was created,
   // or we used an existing tabbed browser.
   //
   // If successful, this begins loading tabs and deletes itself when all tabs
   // have been loaded.
   //
@@ skipping 56 matching lines @@
 #endif
     if (synchronous_) {
       // See comment above windows_ as to why we don't process immediately.
       windows_.swap(windows.get());
       active_window_id_ = active_window_id;
       CHECK(!quit_closure_for_sync_restore_.is_null());
       quit_closure_for_sync_restore_.Run();
       return;
     }
 
-    ProcessSessionWindows(&windows.get(), active_window_id);
+    ProcessSessionWindowsAndNotify(&windows.get(), active_window_id);
+  }
+
+  Browser* ProcessSessionWindowsAndNotify(
+      std::vector<sessions::SessionWindow*>* windows,
+      SessionID::id_type active_window_id) {
+    std::vector<WebContents*> contents;
+    Browser* result =
+        ProcessSessionWindows(windows, active_window_id, &contents);
+    on_session_restored_callbacks_->Notify(static_cast<int>(contents.size()));
+    return result;
   }
 
   Browser* ProcessSessionWindows(std::vector<sessions::SessionWindow*>* windows,
-                                 SessionID::id_type active_window_id) {
+                                 SessionID::id_type active_window_id,
+                                 std::vector<WebContents*>* created_contents) {
     DVLOG(1) << "ProcessSessionWindows " << windows->size();
     base::TimeDelta time_to_process_sessions =
         base::TimeTicks::Now() - restore_started_;
     UMA_HISTOGRAM_CUSTOM_TIMES(
         "SessionRestore.TimeToProcessSessions",
         time_to_process_sessions,
         base::TimeDelta::FromMilliseconds(10),
         base::TimeDelta::FromSeconds(1000),
         100);
 
@@ skipping 76 matching lines @@
         --initial_tab_count;
       int selected_tab_index =
           initial_tab_count > 0 ? browser->tab_strip_model()->active_index()
                                 : std::max(0,
                                     std::min((*i)->selected_tab_index,
                                     static_cast<int>((*i)->tabs.size()) - 1));
       if ((*i)->window_id.id() == active_window_id)
         browser_to_activate = browser;
 
       RestoreTabsToBrowser(*(*i), browser, initial_tab_count,
-                           selected_tab_index);
+                           selected_tab_index, created_contents);
       NotifySessionServiceOfRestoredTabs(browser, initial_tab_count);
       // This needs to be done after restore because closing the last tab will
       // close the whole window.
       if (close_active_tab)
         chrome::CloseWebContents(browser, active_tab, true);
     }
 
     if (browser_to_activate && browser_to_activate->is_type_tabbed())
       last_browser = browser_to_activate;
 
@@ skipping 38 matching lines @@
           extension_misc::APP_LAUNCH_SESSION_RESTORE,
           extension->GetType());
     }
   }
 
   // Adds the tabs from |window| to |browser|. Normal tabs go after the existing
   // tabs but pinned tabs will be pushed in front.
   // If there are no existing tabs, the tab at |selected_tab_index| will be
   // selected. Otherwise, the tab selection will remain untouched.
   void RestoreTabsToBrowser(const sessions::SessionWindow& window,
-                           Browser* browser,
-                           int initial_tab_count,
-                           int selected_tab_index) {
+                            Browser* browser,
+                            int initial_tab_count,
+                            int selected_tab_index,
+                            std::vector<WebContents*>* created_contents) {

[Mr4D (OOO till 08-26), 2015/03/20 16:22:38]

I did look over the code several times and I am not sure why you need this list.
It appears to be a bit overkill since you only look for the number of restored
tabs. Wouldn't it be better to return the number of tabs restored instead?

[sky, 2015/03/20 16:35:10]

Ya, sorry, I should have explained. TabLoader is going to be changed to get this
vector. That's why I didn't go with a simple int*.

     DVLOG(1) << "RestoreTabsToBrowser " << window.tabs.size();
     DCHECK(!window.tabs.empty());
     if (initial_tab_count == 0) {
       for (int i = 0; i < static_cast<int>(window.tabs.size()); ++i) {
         const sessions::SessionTab& tab = *(window.tabs[i]);
 
         // Loads are scheduled for each restored tab unless the tab is going to
         // be selected as ShowBrowser() will load the selected tab.
         bool is_selected_tab = (i == selected_tab_index);
         WebContents* restored_tab =
             RestoreTab(tab, i, browser, is_selected_tab);
 
         // RestoreTab can return NULL if |tab| doesn't have valid data.
         if (!restored_tab)
           continue;
 
+        created_contents->push_back(restored_tab);
+
         // If this isn't the selected tab, there's nothing else to do.
         if (!is_selected_tab)
           continue;
 
         ShowBrowser(
             browser,
             browser->tab_strip_model()->GetIndexOfWebContents(restored_tab));
         // TODO(sky): remove. For debugging 368236.
         CHECK_EQ(browser->tab_strip_model()->GetActiveWebContents(),
                  restored_tab);
         tab_loader_->TabIsLoading(&browser->tab_strip_model()
                                        ->GetActiveWebContents()
                                        ->GetController());
       }
     } else {
       // If the browser already has tabs, we want to restore the new ones after
       // the existing ones. E.g. this happens in Win8 Metro where we merge
       // windows or when launching a hosted app from the app launcher.
       int tab_index_offset = initial_tab_count;
       for (int i = 0; i < static_cast<int>(window.tabs.size()); ++i) {
         const sessions::SessionTab& tab = *(window.tabs[i]);
         // Always schedule loads as we will not be calling ShowBrowser().
-        RestoreTab(tab, tab_index_offset + i, browser, false);
+        WebContents* contents =
+            RestoreTab(tab, tab_index_offset + i, browser, false);
+        if (contents)
+          created_contents->push_back(contents);
       }
     }
   }
 
   // |tab_index| is ignored for pinned tabs which will always be pushed behind
   // the last existing pinned tab.
   // |tab_loader_| will schedule this tab for loading if |is_selected_tab| is
   // false.
   WebContents* RestoreTab(const sessions::SessionTab& tab,
                           const int tab_index,
@@ skipping 280 matching lines @@
 // static
 SessionRestore::CallbackSubscription
     SessionRestore::RegisterOnSessionRestoredCallback(
         const base::Callback<void(int)>& callback) {
   return on_session_restored_callbacks()->Add(callback);
 }
 
 // static
 base::CallbackList<void(int)>*
     SessionRestore::on_session_restored_callbacks_ = nullptr;