Split TabRestoreService into InMemoryTRS and PersistentTRS

chrome/browser/sessions/persistent_tab_restore_service.h

Index: chrome/browser/sessions/persistent_tab_restore_service.h
diff --git a/chrome/browser/sessions/persistent_tab_restore_service.h b/chrome/browser/sessions/persistent_tab_restore_service.h
new file mode 100644
index 0000000000000000000000000000000000000000..193c340bca08f565bc766a0c2732103015e8c714
--- /dev/null
+++ b/chrome/browser/sessions/persistent_tab_restore_service.h
@@ -0,0 +1,113 @@
+// 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.
+
+#include <vector>

[sky, 2012/10/17 21:49:31]

Make sure you svn cp so that history is preserved.

[Philippe, 2012/10/18 09:41:41]

I'm using git which doesn't provide a way to keep track of copies/moves other
than detecting similarities (AFAICT). This file is unfortunately too different
from the original one.

I see two options:
- Keep a single CL but make the change in multiple patch sets (e.g. the first
one would copy the file(s)), so that you can easily see changes by diffing
between patch sets. The downside is that we would loose the history at the SCM
level as you pointed out.

- Make a first CL/commit that copies the file(s) without adding them to the GYP
file. Then I can create another CL changing these new files.

What do you think?

[Philippe, 2012/10/18 09:54:06]

I think I was wrong on this point.

git cl upload --similarity=10 seems to work fine. So I will keep uploading patch
sets on this CL.

+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/ref_counted.h"
+#include "base/time.h"
+#include "chrome/browser/sessions/base_session_service.h"
+#include "chrome/browser/sessions/in_memory_tab_restore_service.h"
+
+class Profile;
+
+class PersistentTabRestoreService : public BaseSessionService,

[sky, 2012/10/17 21:49:31]

Doesn't this result in diamond inheritance? PersistantTabRestoreService gets
ProfileKeyedService from both BaseSessionService and InMemoryTabRestoreService.
Also, I don't like having the two inherit from each other. Refactor the common
code into its' own class and have each instance create and delegate to an
instance.

[Philippe, 2012/10/18 09:41:41]

Sure.

+                                    public InMemoryTabRestoreService {
+public:
+  // Creates a new TabRestoreService and provides an object that provides the
+  // current time. The TabRestoreService does not take ownership of the
+  // |time_factory_|.
+  PersistentTabRestoreService(Profile* profile,
+                              TimeFactory* time_factory = NULL);
+
+  PersistentTabRestoreService();
+  virtual ~PersistentTabRestoreService();
+
+private:
+  // BaseSessionService:
+  virtual void Save() OVERRIDE;
+
+  // InMemoryTabRestoreService:
+  virtual bool IsLoaded() const OVERRIDE;
+  virtual void Shutdown() OVERRIDE;
+  virtual void OnClearEntries() OVERRIDE;
+  virtual void OnRestoreEntryById(
+      SessionID::id_type id,
+      Entries::const_iterator entry_iterator) OVERRIDE;
+  virtual void OnAddEntry() OVERRIDE;
+
+  // Returns the index to persist as the selected index. This is the same
+  // as |tab.current_navigation_index| unless the entry at
+  // |tab.current_navigation_index| shouldn't be persisted. Returns -1 if
+  // no valid navigation to persist.
+  int GetSelectedNavigationIndexToPersist(const Tab& tab);
+
+  // Schedules the commands for a tab close. |selected_index| gives the
+  // index of the selected navigation.
+  void ScheduleCommandsForTab(const Tab& tab, int selected_index);
+
+  // Schedules the commands for a window close.
+  void ScheduleCommandsForWindow(const Window& window);
+
+  // Callback from SessionService when we've received the windows from the
+  // previous session. This creates and add entries to |staging_entries_|
+  // and invokes LoadStateChanged. |ignored_active_window| is ignored because
+  // we don't need to restore activation.
+  void OnGotPreviousSession(Handle handle,
+                            std::vector<SessionWindow*>* windows,
+                            SessionID::id_type ignored_active_window);
+
+  // Invoked when we've loaded the session commands that identify the
+  // previously closed tabs. This creates entries, adds them to
+  // staging_entries_, and invokes LoadState.
+  void OnGotLastSessionCommands(
+      Handle handle,
+      scoped_refptr<InternalGetCommandsRequest> request);
+
+  // Populates |loaded_entries| with Entries from |request|.
+  void CreateEntriesFromCommands(
+      scoped_refptr<InternalGetCommandsRequest> request,
+      std::vector<Entry*>* loaded_entries);
+
+  // Creates and add entries to |entries| for each of the windows in |windows|.
+  void CreateEntriesFromWindows(std::vector<SessionWindow*>* windows,
+                                std::vector<Entry*>* entries);
+
+  // If |id_to_entry| contains an entry for |id| the corresponding entry is
+  // deleted and removed from both |id_to_entry| and |entries|. This is used
+  // when creating entries from the backend file.
+  void RemoveEntryByID(SessionID::id_type id,
+                       IDToEntry* id_to_entry,
+                       std::vector<TabRestoreService::Entry*>* entries);
+
+  // Validates all entries in |entries|, deleting any with no navigations.
+  // This also deletes any entries beyond the max number of entries we can
+  // hold.
+  static void ValidateAndDeleteEmptyEntries(std::vector<Entry*>* entries);
+
+  virtual void LoadTabsFromLastSession() OVERRIDE;
+
+  void LoadStateChanged();
+
+  // The number of entries to write.
+  int entries_to_write_;
+
+  // Number of entries we've written.
+  int entries_written_;
+
+  // Whether we've loaded the last session.
+  int load_state_;
+
+  // Results from previously closed tabs/sessions is first added here. When
+  // the results from both us and the session restore service have finished
+  // loading LoadStateChanged is invoked, which adds these entries to
+  // entries_.
+  std::vector<Entry*> staging_entries_;
+
+  // Used when loading previous tabs/session.
+  CancelableRequestConsumer load_consumer_;
+
+  DISALLOW_COPY_AND_ASSIGN(PersistentTabRestoreService);
+};