Completing refactor of startup_browser_creator_impl

chrome/browser/ui/startup/startup_browser_creator_impl.h

 // Copyright (c) 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.
 
 #ifndef CHROME_BROWSER_UI_STARTUP_STARTUP_BROWSER_CREATOR_IMPL_H_
 #define CHROME_BROWSER_UI_STARTUP_STARTUP_BROWSER_CREATOR_IMPL_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 66 matching lines @@
                            DetermineStartupTabs_CommandLine);
   FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorImplTest,
                            DetermineStartupTabs_NewTabPage);
 
   enum class WelcomeRunType {
     NONE,                // Do not inject the welcome page for this run.
     FIRST_TAB,           // Inject the welcome page as the first tab.
     FIRST_RUN_LAST_TAB,  // Inject the welcome page as the last first-run tab.
   };
 
+  enum class BrowserOpenBehavior {
+    NEW,                  // Open in a new browser.
+    SYNCHRONOUS_RESTORE,  // Attempt a synchronous session restore.
+    USE_EXISTING,         // Attempt to add to an existing tabbed browser.
+  };
+
   // Creates a tab for each of the Tabs in |tabs|. If browser is non-null
   // and a tabbed browser, the tabs are added to it. Otherwise a new tabbed
   // browser is created and the tabs are added to it. The browser the tabs
   // are added to is returned, which is either |browser| or the newly created
   // browser.
   Browser* OpenTabsInBrowser(Browser* browser,
                              bool process_startup,
                              const StartupTabs& tabs);
 
   // If the process was launched with the web application command line flags,
@@ skipping 18 matching lines @@
       const std::vector<GURL>& cmd_line_urls);
 
   // Returns the tabs to be shown on startup, based on the policy functions in
   // the given StartupTabProvider, the given tabs passed by the command line,
   // and the interactions between those policies.
   StartupTabs DetermineStartupTabs(const StartupTabProvider& provider,
                                    const StartupTabs& cmd_line_tabs,
                                    bool is_incognito,
                                    bool is_post_crash_launch);
 
+  // Begins an asynchronous session restore if current state allows it (e.g.,
+  // this is not process startup) and SessionService indicates that one is
+  // necessary. Returns true if restore was initiated, or false if launch
+  // should continue (either synchronously, or asynchronously without
+  // restoring).
+  bool MaybeAsyncRestore(const StartupTabs& tabs,
+                         bool process_startup,
+                         bool is_post_crash_launch);
+
+  // Returns a browser displaying the contents of |tabs|. Based on |behavior|,
+  // this may attempt a session restore or create a new browser. May also allow
+  // DOM Storage to begin cleanup once it's clear it is not needed anymore.
+  Browser* RestoreOrCreateBrowser(const StartupTabs& tabs,
+                                  BrowserOpenBehavior behavior,
+                                  uint32_t restore_options,

[Peter Kasting, 2016/10/31 19:24:06]

Nit: Here and in DetermineSynchronousRestoreOptions() below we use a uint32_t to
refer to some nebulous "restore options".  It took me a while to figure out this
is just SessionRestore::Behavior (but expressed as a uint32_t, I guess to
emphasize it could be a bitmask of any of those values.

It would be nice if there was a type alias for this type, defined in
session_restore.h, so that functions could pass that around instead of uint32_t,
and its meaning would be clear.

[tmartino, 2016/11/02 16:34:00]

Ack. Made a note to self to look at this when I make a cleanup pass post-launch,
but I'd rather not let /browser/sessions creep into scope right now.

[Peter Kasting, 2016/11/02 17:00:46]

SGTM

+                                  bool process_startup,
+                                  bool is_post_crash_launch);
+
+  // Determines how the launch flow should obtain a Browser.
+  static BrowserOpenBehavior DetermineBrowserOpenBehavior(

[Peter Kasting, 2016/10/31 19:24:06]

Nit: I suggest placing static methods either above or below non-static methods,
since they're really more like namespaced globals. (2 places)

[tmartino, 2016/11/02 16:34:00]

Agreed. Moved them to be the last non-deprecated functions.

[grt (UTC plus 2), 2016/11/03 09:43:34]

Minor note: the style guide doesn't dictate where static methods should go in
relation to non-static methods
(https://google.github.io/styleguide/cppguide.html#Declaration_Order). I
interpret this to mean "put them where it makes the most sense given the class
as a whole."

[Peter Kasting, 2016/11/03 21:46:42]

Yes; there is no style rule on this.  The majority of Chrome code I've seen
seems to keep them together above or below non-statics, but it's not required.

+      const SessionStartupPref& pref,
+      bool process_startup,
+      bool is_post_crash_launch,
+      bool has_restore_switch,
+      bool has_new_window_switch,
+      bool has_cmd_line_tabs);
+
+  // Populates and returns the relevant bitmask options which must be passed

[Peter Kasting, 2016/10/31 19:24:06]

Nit: No need for "Populates and"

[tmartino, 2016/11/02 16:34:00]

Done.

+  // when restoring a session.
+  static uint32_t DetermineSynchronousRestoreOptions(
+      bool has_create_browser_default,
+      bool has_create_browser_switch,
+      bool was_mac_login_or_resume);
+
   // Adds a Tab to |tabs| for each url in |urls| that doesn't already exist
   // in |tabs|.
   void AddUniqueURLs(const std::vector<GURL>& urls, StartupTabs* tabs);
 
   // Adds any startup infobars to the selected tab of the given browser.
   void AddInfoBarsIfNecessary(
       Browser* browser,
       chrome::startup::IsProcessStartup is_process_startup);
 
   // Records Rappor metrics on startup URLs.
@@ skipping 53 matching lines @@
   const base::FilePath cur_dir_;
   const base::CommandLine& command_line_;
   Profile* profile_;
   StartupBrowserCreator* browser_creator_;
   bool is_first_run_;
   WelcomeRunType welcome_run_type_;
   DISALLOW_COPY_AND_ASSIGN(StartupBrowserCreatorImpl);
 };
 
 #endif  // CHROME_BROWSER_UI_STARTUP_STARTUP_BROWSER_CREATOR_IMPL_H_