Restoring a session should restore window minimization state

chrome/browser/sessions/session_service.cc

 // Copyright (c) 2011 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_service.h"
 
 #include <algorithm>
 #include <limits>
 #include <set>
 #include <vector>
@@ skipping 26 matching lines @@
 #include "content/common/notification_service.h"
 
 #if defined(OS_MACOSX)
 #include "chrome/browser/app_controller_cppsafe_mac.h"
 #endif
 
 using base::Time;
 
 // Identifier for commands written to file.
 static const SessionCommand::id_type kCommandSetTabWindow = 0;
-// kCommandSetWindowBounds is no longer used (it's superseded by
-// kCommandSetWindowBounds2). I leave it here to document what it was.
+// OBSOLETE Superseded by kCommandSetWindowBounds3.
 // static const SessionCommand::id_type kCommandSetWindowBounds = 1;
 static const SessionCommand::id_type kCommandSetTabIndexInWindow = 2;
 static const SessionCommand::id_type kCommandTabClosed = 3;
 static const SessionCommand::id_type kCommandWindowClosed = 4;
 static const SessionCommand::id_type
     kCommandTabNavigationPathPrunedFromBack = 5;
 static const SessionCommand::id_type kCommandUpdateTabNavigation = 6;
 static const SessionCommand::id_type kCommandSetSelectedNavigationIndex = 7;
 static const SessionCommand::id_type kCommandSetSelectedTabInIndex = 8;
 static const SessionCommand::id_type kCommandSetWindowType = 9;
-static const SessionCommand::id_type kCommandSetWindowBounds2 = 10;
+// OBSOLETE Superseded by kCommandSetWindowBounds3. Except for data migration.
+// static const SessionCommand::id_type kCommandSetWindowBounds2 = 10;
 static const SessionCommand::id_type
     kCommandTabNavigationPathPrunedFromFront = 11;
 static const SessionCommand::id_type kCommandSetPinnedState = 12;
 static const SessionCommand::id_type kCommandSetExtensionAppID = 13;
+static const SessionCommand::id_type kCommandSetWindowBounds3 = 14;
 
 // Every kWritesPerReset commands triggers recreating the file.
 static const int kWritesPerReset = 250;
 
 namespace {
 
 // The callback from GetLastSession is internally routed to SessionService
 // first and then the caller. This is done so that the SessionWindows can be
 // recreated from the SessionCommands and the SessionWindows passed to the
 // caller. The following class is used for this.
@@ skipping 24 matching lines @@
 
 struct WindowBoundsPayload2 {
   SessionID::id_type window_id;
   int32 x;
   int32 y;
   int32 w;
   int32 h;
   bool is_maximized;
 };
 
+struct WindowBoundsPayload3 {
+  SessionID::id_type window_id;
+  int32 x;
+  int32 y;
+  int32 w;
+  int32 h;
+  int32 show_state;
+};
+
 struct IDAndIndexPayload {
   SessionID::id_type id;
   int32 index;
 };
 
 typedef IDAndIndexPayload TabIndexInWindowPayload;
 
 typedef IDAndIndexPayload TabNavigationPathPrunedFromBackPayload;
 
 typedef IDAndIndexPayload SelectedNavigationIndexPayload;
@@ skipping 63 matching lines @@
 void SessionService::SetTabWindow(const SessionID& window_id,
                                   const SessionID& tab_id) {
   if (!ShouldTrackChangesToWindow(window_id))
     return;
 
   ScheduleCommand(CreateSetTabWindowCommand(window_id, tab_id));
 }
 
 void SessionService::SetWindowBounds(const SessionID& window_id,
                                      const gfx::Rect& bounds,
-                                     bool is_maximized) {
+                                     ui::WindowShowState show_state) {
   if (!ShouldTrackChangesToWindow(window_id))
     return;
 
-  ScheduleCommand(CreateSetWindowBoundsCommand(window_id, bounds,
-                                               is_maximized));
+  ScheduleCommand(CreateSetWindowBoundsCommand(window_id, bounds, show_state));
 }
 
 void SessionService::SetTabIndexInWindow(const SessionID& window_id,
                                          const SessionID& tab_id,
                                          int new_index) {
   if (!ShouldTrackChangesToWindow(window_id))
     return;
 
   ScheduleCommand(CreateSetTabIndexInWindowCommand(tab_id, new_index));
 }
@@ skipping 453 matching lines @@
   SessionID::id_type payload[] = { window_id.id(), tab_id.id() };
   SessionCommand* command =
       new SessionCommand(kCommandSetTabWindow, sizeof(payload));
   memcpy(command->contents(), payload, sizeof(payload));
   return command;
 }
 
 SessionCommand* SessionService::CreateSetWindowBoundsCommand(
     const SessionID& window_id,
     const gfx::Rect& bounds,
-    bool is_maximized) {
-  WindowBoundsPayload2 payload = { 0 };
+    ui::WindowShowState show_state) {
+  WindowBoundsPayload3 payload = { 0 };
   payload.window_id = window_id.id();
   payload.x = bounds.x();
   payload.y = bounds.y();
   payload.w = bounds.width();
   payload.h = bounds.height();
-  payload.is_maximized = is_maximized;
-  SessionCommand* command = new SessionCommand(kCommandSetWindowBounds2,
+  payload.show_state = show_state;
+  SessionCommand* command = new SessionCommand(kCommandSetWindowBounds3,
                                                sizeof(payload));
   memcpy(command->contents(), &payload, sizeof(payload));
   return command;
 }
 
 SessionCommand* SessionService::CreateSetTabIndexInWindowCommand(
     const SessionID& tab_id,
     int new_index) {
   TabIndexInWindowPayload payload = { 0 };
   payload.id = tab_id.id();
@@ skipping 229 matching lines @@
 
 bool SessionService::CreateTabsAndWindows(
     const std::vector<SessionCommand*>& data,
     std::map<int, SessionTab*>* tabs,
     std::map<int, SessionWindow*>* windows) {
   // If the file is corrupt (command with wrong size, or unknown command), we
   // still return true and attempt to restore what we we can.
 
   for (std::vector<SessionCommand*>::const_iterator i = data.begin();
        i != data.end(); ++i) {
+    const SessionCommand::id_type kCommandSetWindowBounds2 = 10;
     const SessionCommand* command = *i;
 
     switch (command->id()) {
       case kCommandSetTabWindow: {
         SessionID::id_type payload[2];
         if (!command->GetPayload(payload, sizeof(payload)))
           return true;
         GetTab(payload[1], tabs)->window_id.set_id(payload[0]);
         break;
       }
 
+      // This is here for forward migration only.  New data is saved with
+      // |kCommandSetWindowBounds3|.
       case kCommandSetWindowBounds2: {
         WindowBoundsPayload2 payload;
         if (!command->GetPayload(&payload, sizeof(payload)))
           return true;
         GetWindow(payload.window_id, windows)->bounds.SetRect(payload.x,
                                                               payload.y,
                                                               payload.w,
                                                               payload.h);
-        GetWindow(payload.window_id, windows)->is_maximized =
-            payload.is_maximized;
+        GetWindow(payload.window_id, windows)->show_state =
+            payload.is_maximized ?
+                ui::SHOW_STATE_MAXIMIZED : ui::SHOW_STATE_NORMAL;

[sky, 2011/08/19 16:59:54]

Should this be _DEFAULT?

[dhollowa, 2011/08/19 17:58:53]

No, the DEFAULT state is an indeterminate state, used by the clients for
managing restoration logic.  The saved state here is explicit.

On 2011/08/19 16:59:54, sky wrote:

         break;
       }
 
+      case kCommandSetWindowBounds3: {
+        WindowBoundsPayload3 payload;
+        if (!command->GetPayload(&payload, sizeof(payload)))
+          return true;
+        GetWindow(payload.window_id, windows)->bounds.SetRect(payload.x,
+                                                              payload.y,
+                                                              payload.w,
+                                                              payload.h);
+        ui::WindowShowState show_state = ui::SHOW_STATE_DEFAULT;
+        switch (payload.show_state) {

[sky, 2011/08/19 16:59:54]

How about defining a min/max value and making sure payload.show_state is within
that range?

[dhollowa, 2011/08/19 17:58:53]

Done.

+          case 1:
+            show_state = ui::SHOW_STATE_NORMAL;
+            break;
+          case 2:
+            show_state = ui::SHOW_STATE_MINIMIZED;
+            break;
+          case 3:
+            show_state = ui::SHOW_STATE_MAXIMIZED;
+            break;
+          default:
+            show_state = ui::SHOW_STATE_NORMAL;
+            NOTREACHED();
+            break;
+        }
+        GetWindow(payload.window_id, windows)->show_state = show_state;
+        break;
+      }
+
       case kCommandSetTabIndexInWindow: {
         TabIndexInWindowPayload payload;
         if (!command->GetPayload(&payload, sizeof(payload)))
           return true;
         GetTab(payload.id, tabs)->tab_visual_index = payload.index;
         break;
       }
 
       case kCommandTabClosed:
       case kCommandWindowClosed: {
@@ skipping 168 matching lines @@
 }
 
 void SessionService::BuildCommandsForBrowser(
     Browser* browser,
     std::vector<SessionCommand*>* commands,
     IdToRange* tab_to_available_range,
     std::set<SessionID::id_type>* windows_to_track) {
   DCHECK(browser && commands);
   DCHECK(browser->session_id().id());
 
+  ui::WindowShowState show_state = ui::SHOW_STATE_DEFAULT;

[sky, 2011/08/19 16:59:54]

nit: make this normal and remove last else clause.

[dhollowa, 2011/08/19 17:58:53]

Done.

+  if (browser->window()->IsMaximized())
+    show_state = ui::SHOW_STATE_MAXIMIZED;
+  else if (browser->window()->IsMinimized())
+    show_state = ui::SHOW_STATE_MINIMIZED;
+  else
+    show_state = ui::SHOW_STATE_NORMAL;
+
   commands->push_back(
       CreateSetWindowBoundsCommand(browser->session_id(),
                                    browser->window()->GetRestoredBounds(),
-                                   browser->window()->IsMaximized()));
+                                   show_state));
 
   commands->push_back(CreateSetWindowTypeCommand(
       browser->session_id(), WindowTypeForBrowserType(browser->type())));
 
   bool added_to_windows_to_track = false;
   for (int i = 0; i < browser->tab_count(); ++i) {
     TabContentsWrapper* tab = browser->GetTabContentsWrapperAt(i);
     DCHECK(tab);
     if (tab->profile() == profile() || profile() == NULL) {
       BuildCommandsForTab(browser->session_id(), tab, i,
@@ skipping 322 matching lines @@
       50);
   if (use_long_period) {
     std::string long_name_("SessionRestore.SaveLongPeriod");
     UMA_HISTOGRAM_CUSTOM_TIMES(long_name_,
         delta,
         save_delay_in_mins_,
         save_delay_in_hrs_,
         50);
   }
 }