Sessions API - previously Session Restore API. Supports restoring currently open foreign windows an…

chrome/browser/extensions/api/sessions/sessions_api.cc

Index: chrome/browser/extensions/api/sessions/sessions_api.cc
diff --git a/chrome/browser/extensions/api/session_restore/session_restore_api.cc b/chrome/browser/extensions/api/sessions/sessions_api.cc
similarity index 25%
rename from chrome/browser/extensions/api/session_restore/session_restore_api.cc
rename to chrome/browser/extensions/api/sessions/sessions_api.cc
index ed0042cb6efc7676a160159f8c400df72ba3630c..ce6584a1f7042c79198633b9ae930021f678d320 100644
--- a/chrome/browser/extensions/api/session_restore/session_restore_api.cc
+++ b/chrome/browser/extensions/api/sessions/sessions_api.cc
@@ -1,8 +1,10 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 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/extensions/api/session_restore/session_restore_api.h"
+#include <iostream>
+
+#include "chrome/browser/extensions/api/sessions/sessions_api.h"
 
 #include <vector>
 
@@ -10,8 +12,14 @@
 #include "base/lazy_instance.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/time/time.h"
+#include "chrome/browser/extensions/api/sessions/foreign_session_id.h"
+#include "chrome/browser/extensions/api/tabs/windows_helper.h"
+#include "chrome/browser/extensions/extension_function_dispatcher.h"
 #include "chrome/browser/extensions/extension_function_registry.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
+#include "chrome/browser/extensions/window_controller.h"
+#include "chrome/browser/extensions/window_controller_list.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sessions/session_restore.h"
 #include "chrome/browser/sessions/tab_restore_service_delegate.h"
@@ -25,107 +33,161 @@
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/common/error_utils.h"
 #include "ui/base/layout.h"
 
-
 namespace {
 
-const unsigned int kMaxRecentlyClosedSessionResults = 25;
-const char kRecentlyClosedListEmpty[] =
+const int kMaxRecentlyClosedSessionResults = 25;
+const int kMaxSyncedSessionResults = 10;
+const char kRecentlyClosedListEmptyError[] =

[not at google - send to devlin, 2013/08/12 23:51:34]

this name is bugging me. kNoRecentlyClosedSessionsError.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

     "There are no recently closed sessions.";
-const char kInvalidSessionId[] = "Invalid session id.";
+const char kInvalidSessionIdError[] = "Invalid session id.";

[not at google - send to devlin, 2013/08/12 23:51:34]

would be nicer if it included the session ID in here.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

 const char kNoBrowserToRestoreSession[] =
     "There are no browser windows to restore the session.";
+const char kSessionSyncNotEnabledError[] = "Syncing sessions is not enabled.";
+const char kSyncedSessionsListEmptyError[] = "There are no foreign sessions.";

[not at google - send to devlin, 2013/08/12 23:51:34]

not used

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+const char kSessionNotFoundError[] = "Session is not found.";

[not at google - send to devlin, 2013/08/12 23:51:34]

would be nicer if it included the session ID in here.

[Kristen Dwan, 2013/08/15 07:11:56]

no reason to have session not found and invalid session errors. obviously if
it's not found .. it's invalid. so i put them into one.

+const char kSessionSyncError[] = "Session sync error.";
+
+// Comparator function for use with std::sort that will sort sessions by
+// descending modified_time (i.e., most recent first).
+bool SortSessionsByRecency(const browser_sync::SyncedSession* s1,
+                           const browser_sync::SyncedSession* s2) {
+  return s1->modified_time > s2->modified_time;
+}
 
-}  // namespace
-
-namespace extensions {
-
-namespace GetRecentlyClosed = api::session_restore::GetRecentlyClosed;
-namespace Restore = api::session_restore::Restore;
-namespace tabs = api::tabs;
-namespace windows = api::windows;
-namespace session_restore = api::session_restore;
+scoped_ptr<extensions::api::tabs::Tab> CreateTabModelHelper(

[not at google - send to devlin, 2013/08/12 23:51:34]

Put this whole file in the extensions namespace. Then you don't need to qualify
any of these.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+    const sessions::SerializedNavigationEntry& current_navigation,
+    const std::string& session_id,
+    int index,
+    bool pinned,
+    int selected_index,
+    const extensions::Extension* extension) {
+  scoped_ptr<extensions::api::tabs::Tab> tab_struct(
+      new extensions::api::tabs::Tab);
 
-scoped_ptr<tabs::Tab> SessionRestoreGetRecentlyClosedFunction::CreateTabModel(
-    const TabRestoreService::Tab& tab, int selected_index) {
-  scoped_ptr<tabs::Tab> tab_struct(new tabs::Tab);
-  const sessions::SerializedNavigationEntry& current_navigation =
-      tab.navigations[tab.current_navigation_index];
   GURL gurl = current_navigation.virtual_url();
   std::string title = UTF16ToUTF8(current_navigation.title());
 
+  tab_struct->session_id.reset(new std::string(session_id));
   tab_struct->url.reset(new std::string(gurl.spec()));
   tab_struct->title.reset(new std::string(title.empty() ? gurl.spec() : title));

[not at google - send to devlin, 2013/08/12 23:51:34]

This line looked suspicious so I did some digging.

The way that the normal Tab structure gets populated is via
WebContents::GetTitle, which does a bunch of stuff, including - if the title is
empty - delegating to the current NavigationEntry's "display URL", which in turn
calls net::FormatUrl.

We should do the same. See here:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/aut...

Though you can just use the shorter version of that which takes only the URL and
accept-languages.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

-  tab_struct->index = tab.tabstrip_index;
-  tab_struct->pinned = tab.pinned;
-  tab_struct->id = tab.id;
-  tab_struct->window_id = tab.browser_id;
-  tab_struct->index = tab.tabstrip_index;
-  tab_struct->pinned = tab.pinned;
-  tab_struct->selected = tab.tabstrip_index == selected_index;
+  tab_struct->index = index;
+  tab_struct->pinned = pinned;
+  tab_struct->selected = index == selected_index;
   tab_struct->active = false;
   tab_struct->highlighted = false;
   tab_struct->incognito = false;
-  ExtensionTabUtil::ScrubTabForExtension(GetExtension(),
-                                         tab_struct.get());
+  ExtensionTabUtil::ScrubTabForExtension(extension, tab_struct.get());
   return tab_struct.Pass();
 }
 
+scoped_ptr<extensions::api::windows::Window> CreateWindowModelHelper(
+    std::vector<linked_ptr<extensions::api::tabs::Tab> >* tabs,

[not at google - send to devlin, 2013/08/12 23:51:34]

since you're passing ownership to this method, tabs should be a scoped_ptr

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+    const std::string& session_id,
+    const extensions::api::windows::Window::Type& type,
+    const extensions::api::windows::Window::State& state) {
+  scoped_ptr<extensions::api::windows::Window> window_struct(
+      new extensions::api::windows::Window);
+  window_struct->tabs.reset(tabs);
+  window_struct->session_id.reset(new std::string(session_id));
+  window_struct->incognito = false;
+  window_struct->always_on_top = false;
+  window_struct->focused = false;
+  window_struct->type = type;
+  window_struct->state = state;
+  return window_struct.Pass();
+}
+
+scoped_ptr<extensions::api::sessions::Session> CreateSessionModelHelper(
+    int last_modified,
+    extensions::api::tabs::Tab* tab,
+    extensions::api::windows::Window* window) {

[not at google - send to devlin, 2013/08/12 23:51:34]

pass scoped_ptrs in here.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+  scoped_ptr<extensions::api::sessions::Session> session_struct(
+      new extensions::api::sessions::Session);
+  session_struct->last_modified = last_modified;
+  if (tab)
+    session_struct->tab.reset(tab);
+  else if (window)
+    session_struct->window.reset(window);

[not at google - send to devlin, 2013/08/12 23:51:34]

else
  NOTREACHED()

?

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+  return session_struct.Pass();
+}
+
+inline bool is_type_window(const TabRestoreService::Entry* entry) {

[not at google - send to devlin, 2013/08/12 23:51:34]

inline doesn't mean anything in this context, compiler can figure it out.

also "is_type_window" looks too vague reading code, is_window_entry perhaps.

in any case, use this everywhere or nowhere. It's used only a couple of times
and there are many other instances of comparing directly to either a TAB or
WINDOW (and if you use everywhere - define an is_tab_entry macro too).

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+  return entry->type == TabRestoreService::WINDOW;
+}
+
+}  // namespace
+
+namespace extensions {
+
+namespace GetRecentlyClosed = api::sessions::GetRecentlyClosed;
+namespace GetDevices = api::sessions::GetDevices;
+namespace Restore = api::sessions::Restore;
+namespace tabs = api::tabs;
+namespace windows = api::windows;
+
+scoped_ptr<tabs::Tab> SessionsGetRecentlyClosedFunction::CreateTabModel(
+    const TabRestoreService::Tab& tab, int session_id, int selected_index) {
+  return CreateTabModelHelper(tab.navigations[tab.current_navigation_index],
+                              base::IntToString(session_id),
+                              tab.tabstrip_index,
+                              tab.pinned,
+                              selected_index,
+                              GetExtension()).Pass();
+}
+
 scoped_ptr<windows::Window>
-    SessionRestoreGetRecentlyClosedFunction::CreateWindowModel(
-        const TabRestoreService::Window& window) {
-  scoped_ptr<windows::Window> window_struct(new windows::Window);
+    SessionsGetRecentlyClosedFunction::CreateWindowModel(
+        const TabRestoreService::Window& window,
+        int session_id) {
   DCHECK(!window.tabs.empty());
 
   scoped_ptr<std::vector<linked_ptr<tabs::Tab> > > tabs(
       new std::vector<linked_ptr<tabs::Tab> >);
   for (size_t i = 0; i < window.tabs.size(); ++i) {
-    tabs->push_back(make_linked_ptr(CreateTabModel(window.tabs[i],
-        window.selected_tab_index).release()));
+    tabs->push_back(make_linked_ptr(
+        CreateTabModel(window.tabs[i], window.tabs[i].id,
+                       window.selected_tab_index).release()));
   }
-  window_struct->tabs.reset(tabs.release());
-  window_struct->incognito = false;
-  window_struct->always_on_top = false;
-  window_struct->focused = false;
-  window_struct->type = windows::Window::TYPE_NORMAL;
-  window_struct->state = windows::Window::STATE_NORMAL;
-  return window_struct.Pass();
+
+  return CreateWindowModelHelper(tabs.release(),
+                                 base::IntToString(session_id),
+                                 windows::Window::TYPE_NORMAL,
+                                 windows::Window::STATE_NORMAL).Pass();
 }
 
-scoped_ptr<session_restore::ClosedEntry>
-    SessionRestoreGetRecentlyClosedFunction::CreateEntryModel(
+scoped_ptr<api::sessions::Session>
+    SessionsGetRecentlyClosedFunction::CreateSessionModel(
         const TabRestoreService::Entry* entry) {
-  scoped_ptr<session_restore::ClosedEntry> entry_struct(
-      new session_restore::ClosedEntry);
-  switch (entry->type) {
-    case TabRestoreService::TAB:
-      entry_struct->tab.reset(CreateTabModel(
-          *static_cast<const TabRestoreService::Tab*>(entry), -1).release());
-      break;
-    case TabRestoreService::WINDOW:
-      entry_struct->window.reset(CreateWindowModel(
-          *static_cast<const TabRestoreService::Window*>(entry)).release());
-      break;
-    default:
-      NOTREACHED();
+  if (entry->type == TabRestoreService::TAB) {

[not at google - send to devlin, 2013/08/12 23:51:34]

I preferred the switch...

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+    scoped_ptr<tabs::Tab> tab(CreateTabModel(
+        *static_cast<const TabRestoreService::Tab*>(entry), entry->id, -1)
+        .release());

[not at google - send to devlin, 2013/08/12 23:51:34]

CreateTabModel already returns a scoped_ptr so you don't need the release().
Just assign.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+    return CreateSessionModelHelper(entry->timestamp.ToTimeT(), tab.release(),
+                                    NULL).Pass();
   }
-  entry_struct->timestamp = entry->timestamp.ToTimeT();
-  entry_struct->id = entry->id;
-  return entry_struct.Pass();
+
+  DCHECK(entry->type == TabRestoreService::WINDOW);
+  scoped_ptr<windows::Window> window(CreateWindowModel(
+      *static_cast<const TabRestoreService::Window*>(entry), entry->id)
+      .release());
+  return CreateSessionModelHelper(entry->timestamp.ToTimeT(), NULL,
+                                  window.release()).Pass();
 }
 
-bool SessionRestoreGetRecentlyClosedFunction::RunImpl() {
+bool SessionsGetRecentlyClosedFunction::RunImpl() {
   scoped_ptr<GetRecentlyClosed::Params> params(
       GetRecentlyClosed::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  unsigned int max_results = kMaxRecentlyClosedSessionResults;
+  int max_results = kMaxRecentlyClosedSessionResults;
   if (params->options && params->options->max_results)
     max_results = *params->options->max_results;
   EXTENSION_FUNCTION_VALIDATE(max_results >= 0 &&
                               max_results <= kMaxRecentlyClosedSessionResults);
 
-  std::vector<linked_ptr<session_restore::ClosedEntry> > result;
+  std::vector<linked_ptr<api::sessions::Session> > result;
   TabRestoreService* tab_restore_service =
       TabRestoreServiceFactory::GetForProfile(profile());
   DCHECK(tab_restore_service);
@@ -135,12 +197,13 @@ bool SessionRestoreGetRecentlyClosedFunction::RunImpl() {
   // uninteresting entries.
   TabRestoreService::Entries entries = tab_restore_service->entries();
   for (TabRestoreService::Entries::const_iterator it = entries.begin();
-       it != entries.end() && result.size() < max_results; ++it) {
+       it != entries.end() && static_cast<int>(result.size()) < max_results;
+       ++it) {
     TabRestoreService::Entry* entry = *it;
     if (!params->options || params->options->entry_type ==
         GetRecentlyClosed::Params::Options::ENTRY_TYPE_NONE) {
       // Include both tabs and windows if type is not defined.
-      result.push_back(make_linked_ptr(CreateEntryModel(entry).release()));
+      result.push_back(make_linked_ptr(CreateSessionModel(entry).release()));
     } else if (
         (params->options->entry_type ==
             GetRecentlyClosed::Params::Options::ENTRY_TYPE_TAB &&
@@ -148,7 +211,7 @@ bool SessionRestoreGetRecentlyClosedFunction::RunImpl() {
         (params->options->entry_type ==
             GetRecentlyClosed::Params::Options::ENTRY_TYPE_WINDOW &&
                 entry->type == TabRestoreService::WINDOW)) {
-      result.push_back(make_linked_ptr(CreateEntryModel(entry).release()));
+      result.push_back(make_linked_ptr(CreateSessionModel(entry).release()));
     }
   }
 
@@ -156,54 +219,223 @@ bool SessionRestoreGetRecentlyClosedFunction::RunImpl() {
   return true;
 }
 
-bool SessionRestoreRestoreFunction::RunImpl() {
-  scoped_ptr<Restore::Params> params(Restore::Params::Create(*args_));
+scoped_ptr<tabs::Tab> SessionsGetDevicesFunction::CreateTabModel(
+    const std::string& session_tag,
+    const SessionTab& tab,
+    int tab_index,
+    int selected_index) {
+  std::string session_id =
+      ForeignSessionId(session_tag, tab.tab_id.id()).ToString();

[not at google - send to devlin, 2013/08/12 23:51:34]

palindromic expression, nice.

+  return CreateTabModelHelper(tab.navigations[tab.current_navigation_index],
+                              session_id,
+                              tab_index,
+                              tab.pinned,
+                              selected_index,
+                              GetExtension()).Pass();

[not at google - send to devlin, 2013/08/12 23:51:34]

I don't think you need to Pass() if the scoped_ptr is already an rvalue? But I'm
just as likely to be wrong.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+}
+
+scoped_ptr<windows::Window> SessionsGetDevicesFunction::CreateWindowModel(
+        const SessionWindow& window, const std::string& session_tag) {
+  DCHECK(!window.tabs.empty());
+
+  scoped_ptr<std::vector<linked_ptr<tabs::Tab> > > tabs(
+      new std::vector<linked_ptr<tabs::Tab> >);
+  for (size_t i = 0; i < window.tabs.size(); ++i) {
+    tabs->push_back(make_linked_ptr(
+        CreateTabModel(session_tag, *window.tabs[i], i,
+                       window.selected_tab_index).release()));
+  }
+
+  std::string session_id =
+      ForeignSessionId(session_tag, window.window_id.id()).ToString();
+
+  windows::Window::Type type;
+  switch (window.type) {
+    case Browser::TYPE_TABBED:
+      type = windows::Window::TYPE_NORMAL;
+      break;
+    case Browser::TYPE_POPUP:
+      type = windows::Window::TYPE_POPUP;
+      break;
+    default:

[not at google - send to devlin, 2013/08/12 23:51:34]

default is unnecessary. Just initialize type to TYPE_NONE. let compiler catch
errors.

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+      type = windows::Window::TYPE_NONE;
+  }
+
+  windows::Window::State state;
+  switch (window.show_state) {
+    case ui::SHOW_STATE_NORMAL:
+      state = windows::Window::STATE_NORMAL;
+      break;
+    case ui::SHOW_STATE_MINIMIZED:
+      state = windows::Window::STATE_MINIMIZED;
+      break;
+    case ui::SHOW_STATE_MAXIMIZED:
+      state = windows::Window::STATE_MAXIMIZED;
+      break;
+    case ui::SHOW_STATE_FULLSCREEN:
+      state = windows::Window::STATE_FULLSCREEN;
+      break;
+    default:
+      state = windows::Window::STATE_NONE;

[not at google - send to devlin, 2013/08/12 23:51:34]

ditto

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+  }
+
+  scoped_ptr<windows::Window> window_struct(
+      CreateWindowModelHelper(tabs.release(), session_id, type, state)
+      .release());

[not at google - send to devlin, 2013/08/12 23:51:34]

release also not needed here

[Kristen Dwan, 2013/08/15 07:11:56]

Done.

+  // TODO(dwankri): Dig deeper to resolve bounds not being optional, so closed
+  // windows in GetRecentlyClosed can have set values in Window helper.
+  window_struct->left.reset(new int(window.bounds.x()));
+  window_struct->top.reset(new int(window.bounds.y()));
+  window_struct->width.reset(new int(window.bounds.width()));
+  window_struct->height.reset(new int(window.bounds.height()));
+
+  return window_struct.Pass();
+}
+
+scoped_ptr<api::sessions::Session>
+SessionsGetDevicesFunction::CreateSessionModel(
+    const SessionWindow& window, const std::string& session_tag) {
+  return CreateSessionModelHelper(window.timestamp.ToTimeT(), NULL,
+                                  CreateWindowModel(window, session_tag)
+                                  .release()).Pass();
+}
+
+scoped_ptr<api::sessions::Device> SessionsGetDevicesFunction::CreateDeviceModel(
+    const browser_sync::SyncedSession* session) {
+  scoped_ptr<api::sessions::Device> device_struct(new api::sessions::Device);
+  device_struct->info = session->session_name;
+
+  for (browser_sync::SyncedSession::SyncedWindowMap::const_iterator it =
+       session->windows.begin(); it != session->windows.end(); ++it) {
+    device_struct->sessions.push_back(make_linked_ptr(CreateSessionModel(
+        *it->second, session->session_tag).release()));
+  }
+  return device_struct.Pass();
+}
+
+bool SessionsGetDevicesFunction::RunImpl() {
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile());
+  if (!(service && service->GetPreferredDataTypes().Has(syncer::SESSIONS))) {
+    // Sync not enabled.
+    results_ = GetDevices::Results::Create(
+        std::vector<linked_ptr<api::sessions::Device> >());
+    return true;
+  }
+
+  browser_sync::SessionModelAssociator* associator =
+      service->GetSessionModelAssociator();
+  std::vector<const browser_sync::SyncedSession*> sessions;
+  if (!(associator && associator->GetAllForeignSessions(&sessions))) {
+    results_ = GetDevices::Results::Create(
+        std::vector<linked_ptr<api::sessions::Device> >());
+    return true;
+  }
+
+  int max_results = kMaxSyncedSessionResults;
+  scoped_ptr<GetDevices::Params> params(GetDevices::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
+  if (params->options && params->options->max_results)
+    max_results = *params->options->max_results.get();
+  EXTENSION_FUNCTION_VALIDATE(max_results >= 0 &&
+                              max_results <= kMaxSyncedSessionResults);
+
+  std::vector<linked_ptr<api::sessions::Device> > result;
+  // Sort sessions from most recent to least recent.
+  std::sort(sessions.begin(), sessions.end(), SortSessionsByRecency);
+  for (size_t i = 0; i < sessions.size() &&
+      static_cast<int>(result.size()) < max_results; ++i) {
+    result.push_back(make_linked_ptr(CreateDeviceModel(sessions[i]).release()));
+  }
 
-  Browser* browser =
-      chrome::FindBrowserWithProfile(profile(),
-                                     chrome::HOST_DESKTOP_TYPE_NATIVE);
-  if (!browser) {
-    error_ = kNoBrowserToRestoreSession;
+  results_ = GetDevices::Results::Create(result);
+  return true;
+}
+
+void SessionsRestoreFunction::SetResultRestoredTab(
+    const content::WebContents* contents) {
+  scoped_ptr<tabs::Tab> tab(tabs::Tab::FromValue(
+      *ExtensionTabUtil::CreateTabValue(contents, GetExtension())));
+  results_ = Restore::Results::Create(*CreateSessionModelHelper(
+      base::Time::Now().ToTimeT(),
+      tab.release(),
+      NULL).get());
+}
+
+bool SessionsRestoreFunction::SetResultRestoredWindow(int window_id) {
+    WindowController* controller = NULL;
+  if (!GetWindowFromWindowID(this, window_id, &controller)) {
+    // error_ is set by GetWindowFromWindowId function call.
     return false;
   }
+  scoped_ptr<windows::Window> window(windows::Window::FromValue(
+      *controller->CreateWindowValueWithTabs(GetExtension())));
+  results_ = Restore::Results::Create(*CreateSessionModelHelper(
+      base::Time::Now().ToTimeT(),
+      NULL,
+      window.release()).get());
+  return true;
+}
 
+bool SessionsRestoreFunction::RestoreMostRecentlyClosed(Browser* browser) {
   TabRestoreService* tab_restore_service =
       TabRestoreServiceFactory::GetForProfile(profile());
+  chrome::HostDesktopType host_desktop_type = browser->host_desktop_type();
+  TabRestoreService::Entries entries = tab_restore_service->entries();
+
+  if (entries.empty()) {
+    SetError(kRecentlyClosedListEmptyError);
+    return false;
+  }
+
+  bool is_window = is_type_window(entries.front());
   TabRestoreServiceDelegate* delegate =
       TabRestoreServiceDelegate::FindDelegateForWebContents(
           browser->tab_strip_model()->GetActiveWebContents());
-  DCHECK(delegate);
+  std::vector<content::WebContents*> contents =
+      tab_restore_service->RestoreMostRecentEntry(delegate, host_desktop_type);
+  DCHECK(contents.size());
+
+  if (is_window) {
+    return SetResultRestoredWindow(
+        ExtensionTabUtil::GetWindowIdOfTab(contents[0]));
+  }
+
+  SetResultRestoredTab(contents[0]);
+  return true;
+}
+
+bool SessionsRestoreFunction::RestoreLocalSession(int id, Browser* browser) {
+  TabRestoreService* tab_restore_service =
+      TabRestoreServiceFactory::GetForProfile(profile());
   chrome::HostDesktopType host_desktop_type = browser->host_desktop_type();
   TabRestoreService::Entries entries = tab_restore_service->entries();
 
   if (entries.empty()) {
-    error_ = kRecentlyClosedListEmpty;
+    SetError(kInvalidSessionIdError);

[not at google - send to devlin, 2013/08/12 23:51:34]

Doesn't seem like the right error string.

[Kristen Dwan, 2013/08/15 07:11:56]

i figured if you passed in an id and sessions are empty .. you obviously passed
an invalid id. but i could make it kNoRecentlyClosedSessionsError as well. but
it seemed more fitting to say - look, this id is invalid.

[not at google - send to devlin, 2013/08/15 20:16:24]

Makes sense.

     return false;
   }
 
-  if (!params->id) {
-    tab_restore_service->RestoreMostRecentEntry(delegate, host_desktop_type);
-    return true;
-  }
-
   // Check if the recently closed list contains an entry with the provided id.
   bool is_valid_id = false;
+  bool is_window = false;
   for (TabRestoreService::Entries::iterator it = entries.begin();
        it != entries.end(); ++it) {
-    if ((*it)->id == *params->id) {
+    if ((*it)->id == id) {
       is_valid_id = true;
+      // The only time a full window is being restored is if the entry ID
+      // matches the provided ID and the entry type is Window.
+      is_window = is_type_window(*it);
       break;
     }
-
     // For Window entries, see if the ID matches a tab. If so, report true for
     // the window as the Entry.
-    if ((*it)->type == TabRestoreService::WINDOW) {
+    if (is_type_window(*it)) {
       std::vector<TabRestoreService::Tab>& tabs =
           static_cast<TabRestoreService::Window*>(*it)->tabs;
       for (std::vector<TabRestoreService::Tab>::iterator tab_it = tabs.begin();
            tab_it != tabs.end(); ++tab_it) {
-        if ((*tab_it).id == *params->id) {
+        if ((*tab_it).id == id) {
           is_valid_id = true;
           break;
         }
@@ -212,27 +444,125 @@ bool SessionRestoreRestoreFunction::RunImpl() {
   }
 
   if (!is_valid_id) {
-    error_ = kInvalidSessionId;
+    SetError(kInvalidSessionIdError);

[not at google - send to devlin, 2013/08/12 23:51:34]

doesn't seem like the right error string either.

[Kristen Dwan, 2013/08/15 07:11:56]

if you didn't find the specific id, it would be invalid..?

[not at google - send to devlin, 2013/08/15 20:16:24]

Makes sense, sorry.

     return false;
   }
 
-  tab_restore_service->RestoreEntryById(delegate, *params->id,
-      host_desktop_type, UNKNOWN);
+  TabRestoreServiceDelegate* delegate =
+      TabRestoreServiceDelegate::FindDelegateForWebContents(
+          browser->tab_strip_model()->GetActiveWebContents());
+  std::vector<content::WebContents*> contents =
+      tab_restore_service->RestoreEntryById(delegate, id, host_desktop_type,
+      UNKNOWN);
+  // We know the entry is valid. It should always be returned by contents.
+  DCHECK(contents.size());

[not at google - send to devlin, 2013/08/12 23:51:34]

I see. Yeah maybe we should just rely on this check and not bother at all about
iterating through the entries apart from finding is_window. Might as well let
the library do this for us - unless for some reason it's not doing it for us.

[Kristen Dwan, 2013/08/15 07:11:56]

good point. it does the exact thing. but before there was no way of knowing what
happened since the call to restore was void. updated to only check for if it's a
window.

+
+  // The restored entry is a full window if ID matches and type is Window.
+  // Retrieve the window through any of the tabs in contents.
+  if (is_window) {
+    return SetResultRestoredWindow(
+        ExtensionTabUtil::GetWindowIdOfTab(contents[0]));
+  }
+
+  SetResultRestoredTab(contents[0]);
   return true;
 }
 
-SessionRestoreAPI::SessionRestoreAPI(Profile* profile) {
+bool SessionsRestoreFunction::RestoreForeignSession(
+    const ForeignSessionId* fs_id,
+    Browser* browser) {
+  ProfileSyncService* service =
+      ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile());
+  if (!(service && service->GetPreferredDataTypes().Has(syncer::SESSIONS))) {
+    SetError(kSessionSyncNotEnabledError);

[not at google - send to devlin, 2013/08/12 23:51:34]

let's just make all of the not-found errors the same, regardless of the cause.

[Kristen Dwan, 2013/08/15 07:11:56]

we can't include the session id in the error message then if combine all, since
it's an optional parameter. in any case i combined sync availability errors into
one. down to 4 different errors in total..

[not at google - send to devlin, 2013/08/15 20:16:24]

Ah makes sense.

+    return false;
+  }
+  browser_sync::SessionModelAssociator* associator =
+      service->GetSessionModelAssociator();
+  if (!associator) {
+    SetError(kSessionSyncError);

[not at google - send to devlin, 2013/08/12 23:51:34]

what does it actually mean for this to be null?

[Kristen Dwan, 2013/08/15 07:11:56]

"Returns the session model associator associated with this type -- (in this case
the type is syncer::SESSIONS) --, but only if the associator is running.  If it
is doing anything else, it will return null."

soo, basically it's null if there exists no data type controller associated with
the model or if the controller isn't in "running" state -- meaning in sync with
the cloud. and if it's null, we have no way of accessing the synced information.

+    return false;
+  }
+
+  const SessionTab* tab = NULL;
+  if (associator->GetForeignTab(fs_id->session_tag(), fs_id->id(), &tab)) {
+    TabStripModel* tab_strip = browser->tab_strip_model();
+    content::WebContents* contents = tab_strip->GetActiveWebContents();
+
+    content::WebContents* tab_contents =
+        SessionRestore::RestoreForeignSessionTab(contents, *tab,
+                                                 NEW_FOREGROUND_TAB);
+    SetResultRestoredTab(tab_contents);
+    return true;
+  }
+
+  // Restoring a full window.
+  std::vector<const SessionWindow*> windows;
+  if (!associator->GetForeignSession(fs_id->session_tag(), &windows)) {
+    SetError(kSessionNotFoundError);
+    return false;
+  }
+
+  std::vector<const SessionWindow*>::const_iterator window = windows.begin();
+  while (window != windows.end() && (*window)->window_id.id() != fs_id->id()) {
+    ++window;
+  }
+  if (window == windows.end()) {
+    SetError(kSessionNotFoundError);
+    return false;
+  }
+
+  chrome::HostDesktopType host_desktop_type = browser->host_desktop_type();
+  // Only restore one window at a time.
+  std::vector<Browser*> browsers =
+      SessionRestore::RestoreForeignSessionWindows(profile(), host_desktop_type,
+                                                   window, window + 1);
+  // Will always create one browser because we only restore one window per call.
+  DCHECK_EQ(1u, browsers.size());
+  return SetResultRestoredWindow(ExtensionTabUtil::GetWindowId(browsers[0]));
+}
+
+bool SessionsRestoreFunction::RunImpl() {
+  scoped_ptr<Restore::Params> params(Restore::Params::Create(*args_));
+  EXTENSION_FUNCTION_VALIDATE(params.get());
+
+  Browser* browser =
+      chrome::FindBrowserWithProfile(profile(),
+                                     chrome::HOST_DESKTOP_TYPE_NATIVE);
+  if (!browser) {
+    SetError(kNoBrowserToRestoreSession);
+    return false;
+  }
+
+  if (!params->session_id)
+    return RestoreMostRecentlyClosed(browser);
+
+  if (ForeignSessionId::IsForeignSessionId(*params->session_id)) {

[not at google - send to devlin, 2013/08/12 23:51:34]

it would be ideal if this foreign vs local stuff were all contained in this
class. you can then check like IsForeign, IsLocal, whatever.

and if you had ForeignSessionId::Parse instead of this (returning a scoped_ptr)
then you'd just null check rather than having that extra isForeignSessionId
method.

That's one advantage of a Parse method, lets you actually signal errors.

[Kristen Dwan, 2013/08/15 07:11:56]

way nicer now :)

+    scoped_ptr<ForeignSessionId> fs_id(
+        new ForeignSessionId(*params->session_id));
+    return RestoreForeignSession(fs_id.get(), browser);
+  }
+
+  int id;
+  if (!base::StringToInt(*params->session_id, &id)) {
+    SetError(kInvalidSessionIdError);
+    return false;
+  }
+  return RestoreLocalSession(id, browser);
+}
+
+SessionsAPI::SessionsAPI(Profile* profile) {
 }
 
-SessionRestoreAPI::~SessionRestoreAPI() {
+SessionsAPI::~SessionsAPI() {
 }
 
-static base::LazyInstance<ProfileKeyedAPIFactory<SessionRestoreAPI> >
+static base::LazyInstance<ProfileKeyedAPIFactory<SessionsAPI> >
     g_factory = LAZY_INSTANCE_INITIALIZER;
 
 // static
-ProfileKeyedAPIFactory<SessionRestoreAPI>*
-    SessionRestoreAPI::GetFactoryInstance() {
+ProfileKeyedAPIFactory<SessionsAPI>*
+    SessionsAPI::GetFactoryInstance() {
   return &g_factory.Get();
 }