Add the window info modification function.

The new ModifyWindowInfo function will be used by
FullRestoreSaveHandler::SaveWindowInfo in CL:2597519 to modify and save
the window information for the restore data.

Modify the unit tests to verify the window info modification function.

Move the position of the AppRestoreData constructor function, and the
field restore_bounds name to consistent with the WindowInfo structure.

BUG=1146900

Change-Id: I0e5755ffdf966d29139be67bf9a9a00a981c7df6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2598084Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840619}

components/full_restore/BUILD.gn

@@ -30,6 +30,7 @@ component("full_restore") {
   public_deps = [
     "//ash/public/cpp",
     "//base",
+    "//chromeos/ui/base:base",
     "//components/account_id:account_id",
     "//components/services/app_service/public/cpp:intents",
     "//components/services/app_service/public/mojom",

components/full_restore/DEPS

 include_rules = [
   "+ash/public",
+  "+chromeos/ui/base/window_state_type.h",
   "+components/account_id/account_id.h",
   "+components/services/app_service/public",
   "+ui",

components/full_restore/app_restore_data.cc

@@ -9,6 +9,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "components/full_restore/app_launch_info.h"
+#include "components/full_restore/window_info.h"
 #include "components/services/app_service/public/cpp/intent_util.h"
 
 namespace full_restore {
@@ -24,7 +25,7 @@ const char kIntentKey[] = "intent";
 const char kFilePathsKey[] = "file_paths";
 const char kActivationIndexKey[] = "index";
 const char kDeskIdKey[] = "desk_id";
-const char kRestoredBoundsKey[] = "restored_bounds";
+const char kRestoreBoundsKey[] = "restore_bounds";
 const char kcurrentBoundsKey[] = "current_bounds";
 const char kWindowStateTypeKey[] = "window_state_type";
 
@@ -106,6 +107,16 @@ base::Optional<gfx::Rect> GetBoundsRectFromDict(
   return gfx::Rect(rect[0], rect[1], rect[2], rect[3]);
 }
 
+// Gets WindowStateType from base::DictionaryValue, e.g. { "window_state_type":
+// 2 } returns WindowStateType::kMinimized.
+base::Optional<chromeos::WindowStateType> GetWindowStateTypeFromDict(
+    const base::DictionaryValue& dict) {
+  return dict.HasKey(kWindowStateTypeKey)
+             ? base::make_optional(static_cast<chromeos::WindowStateType>(
+                   dict.FindIntKey(kWindowStateTypeKey).value()))
+             : base::nullopt;
+}
+
 }  // namespace
 
 AppRestoreData::AppRestoreData() = default;
@@ -126,9 +137,9 @@ AppRestoreData::AppRestoreData(base::Value&& value) {
   file_paths = GetFilePathsFromDict(*data_dict);
   activation_index = GetIntValueFromDict(*data_dict, kActivationIndexKey);
   desk_id = GetIntValueFromDict(*data_dict, kDeskIdKey);
-  restored_bounds = GetBoundsRectFromDict(*data_dict, kRestoredBoundsKey);
+  restore_bounds = GetBoundsRectFromDict(*data_dict, kRestoreBoundsKey);
   current_bounds = GetBoundsRectFromDict(*data_dict, kcurrentBoundsKey);
-  window_state_type = GetIntValueFromDict(*data_dict, kWindowStateTypeKey);
+  window_state_type = GetWindowStateTypeFromDict(*data_dict);
 
   if (data_dict->HasKey(kIntentKey)) {
     intent = apps_util::ConvertValueToIntent(
@@ -136,6 +147,19 @@ AppRestoreData::AppRestoreData(base::Value&& value) {
   }
 }
 
+AppRestoreData::AppRestoreData(std::unique_ptr<AppLaunchInfo> app_launch_info) {
+  if (!app_launch_info)
+    return;
+
+  event_flag = std::move(app_launch_info->event_flag);
+  container = std::move(app_launch_info->container);
+  disposition = std::move(app_launch_info->disposition);
+  display_id = std::move(app_launch_info->display_id);
+  url = std::move(app_launch_info->url);
+  file_paths = std::move(app_launch_info->file_paths);
+  intent = std::move(app_launch_info->intent);
+}
+
 AppRestoreData::~AppRestoreData() = default;
 
 std::unique_ptr<AppRestoreData> AppRestoreData::Clone() const {
@@ -168,8 +192,8 @@ std::unique_ptr<AppRestoreData> AppRestoreData::Clone() const {
   if (desk_id.has_value())
     data->desk_id = desk_id.value();
 
-  if (restored_bounds.has_value())
-    data->restored_bounds = restored_bounds.value();
+  if (restore_bounds.has_value())
+    data->restore_bounds = restore_bounds.value();
 
   if (current_bounds.has_value())
     data->current_bounds = current_bounds.value();
@@ -218,9 +242,9 @@ base::Value AppRestoreData::ConvertToValue() const {
   if (desk_id.has_value())
     launch_info_dict.SetIntKey(kDeskIdKey, desk_id.value());
 
-  if (restored_bounds.has_value()) {
-    launch_info_dict.SetKey(kRestoredBoundsKey,
-                            ConvertRectToValue(restored_bounds.value()));
+  if (restore_bounds.has_value()) {
+    launch_info_dict.SetKey(kRestoreBoundsKey,
+                            ConvertRectToValue(restore_bounds.value()));
   }
 
   if (current_bounds.has_value()) {
@@ -228,36 +252,29 @@ base::Value AppRestoreData::ConvertToValue() const {
                             ConvertRectToValue(current_bounds.value()));
   }
 
-  if (window_state_type.has_value())
-    launch_info_dict.SetIntKey(kWindowStateTypeKey, window_state_type.value());
+  if (window_state_type.has_value()) {
+    launch_info_dict.SetIntKey(kWindowStateTypeKey,
+                               static_cast<int>(window_state_type.value()));
+  }
 
   return launch_info_dict;
 }
 
-AppRestoreData::AppRestoreData(std::unique_ptr<AppLaunchInfo> app_launch_info) {
-  if (!app_launch_info)
-    return;
-
-  if (app_launch_info->event_flag.has_value())
-    event_flag = app_launch_info->event_flag.value();
-
-  if (app_launch_info->container.has_value())
-    container = app_launch_info->container.value();
-
-  if (app_launch_info->disposition.has_value())
-    disposition = app_launch_info->disposition.value();
+void AppRestoreData::ModifyWindowInfo(const WindowInfo& window_info) {
+  if (window_info.activation_index.has_value())
+    activation_index = window_info.activation_index.value();
 
-  if (app_launch_info->display_id.has_value())
-    display_id = app_launch_info->display_id.value();
+  if (window_info.desk_id.has_value())
+    desk_id = window_info.desk_id.value();
 
-  if (app_launch_info->url.has_value())
-    url = std::move(app_launch_info->url.value());
+  if (window_info.restore_bounds.has_value())
+    restore_bounds = std::move(window_info.restore_bounds.value());
 
-  if (app_launch_info->file_paths.has_value())
-    file_paths = std::move(app_launch_info->file_paths.value());
+  if (window_info.current_bounds.has_value())
+    current_bounds = window_info.current_bounds.value();
 
-  if (app_launch_info->intent.has_value())
-    intent = std::move(app_launch_info->intent.value());
+  if (window_info.window_state_type.has_value())
+    window_state_type = window_info.window_state_type.value();
 }
 
 }  // namespace full_restore

components/full_restore/app_restore_data.h

@@ -10,6 +10,7 @@
 
 #include "base/component_export.h"
 #include "base/optional.h"
+#include "chromeos/ui/base/window_state_type.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
 #include "ui/gfx/geometry/rect.h"
 #include "url/gurl.h"
@@ -21,6 +22,7 @@ class Value;
 namespace full_restore {
 
 struct AppLaunchInfo;
+struct WindowInfo;
 
 // This is the struct used by RestoreData to save both app launch parameters and
 // app window information. This struct can be converted to JSON format to be
@@ -28,12 +30,13 @@ struct AppLaunchInfo;
 struct COMPONENT_EXPORT(FULL_RESTORE) AppRestoreData {
   AppRestoreData();
   explicit AppRestoreData(base::Value&& value);
-
-  ~AppRestoreData();
+  explicit AppRestoreData(std::unique_ptr<AppLaunchInfo> app_launch_info);
 
   AppRestoreData(const AppRestoreData&) = delete;
   AppRestoreData& operator=(const AppRestoreData&) = delete;
 
+  ~AppRestoreData();
+
   std::unique_ptr<AppRestoreData> Clone() const;
 
   // Converts the struct LaunchAndWindowInfo to base::Value, e.g.:
@@ -53,7 +56,8 @@ struct COMPONENT_EXPORT(FULL_RESTORE) AppRestoreData {
   // }
   base::Value ConvertToValue() const;
 
-  AppRestoreData(std::unique_ptr<AppLaunchInfo> app_launch_info);
+  // Modify the window's information based on |window_info|.
+  void ModifyWindowInfo(const WindowInfo& window_info);
 
   // App launch parameters.
   base::Optional<int32_t> event_flag;
@@ -67,9 +71,9 @@ struct COMPONENT_EXPORT(FULL_RESTORE) AppRestoreData {
   // Window's information.
   base::Optional<int32_t> activation_index;
   base::Optional<int32_t> desk_id;
-  base::Optional<gfx::Rect> restored_bounds;
+  base::Optional<gfx::Rect> restore_bounds;
   base::Optional<gfx::Rect> current_bounds;
-  base::Optional<int32_t> window_state_type;
+  base::Optional<chromeos::WindowStateType> window_state_type;
 };
 
 }  // namespace full_restore

components/full_restore/restore_data.cc

@@ -9,6 +9,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/values.h"
 #include "components/full_restore/app_launch_info.h"
+#include "components/full_restore/window_info.h"
 
 namespace full_restore {
 
@@ -91,4 +92,18 @@ void RestoreData::AddAppLaunchInfo(
       std::make_unique<AppRestoreData>(std::move(app_launch_info));
 }
 
+void RestoreData::ModifyWindowInfo(const std::string& app_id,
+                                   int32_t id,
+                                   const WindowInfo& window_info) {
+  auto it = app_id_to_launch_list_.find(app_id);
+  if (it == app_id_to_launch_list_.end())
+    return;
+
+  auto data_it = it->second.find(id);
+  if (data_it == it->second.end())
+    return;
+
+  data_it->second->ModifyWindowInfo(window_info);
+}
+
 }  // namespace full_restore

components/full_restore/restore_data.h

@@ -18,6 +18,7 @@ class Value;
 namespace full_restore {
 
 struct AppLaunchInfo;
+struct WindowInfo;
 
 // This class is responsible for saving all app launch and app windows
 // information. It can be converted to JSON format to be written to the
@@ -81,6 +82,12 @@ class COMPONENT_EXPORT(FULL_RESTORE) RestoreData {
   // Add |app_launch_info| to |app_id_to_launch_list_|.
   void AddAppLaunchInfo(std::unique_ptr<AppLaunchInfo> app_launch_info);
 
+  // Modify the window's information based on |window_info| for the window with
+  // |id| of the app with |app_id|.
+  void ModifyWindowInfo(const std::string& app_id,
+                        int32_t id,
+                        const WindowInfo& window_info);
+
   const AppIdToLaunchList& app_id_to_launch_list() const {
     return app_id_to_launch_list_;
   }

components/full_restore/restore_data_unittest.cc

@@ -7,11 +7,14 @@
 #include <memory>
 #include <utility>
 
+#include "chromeos/ui/base/window_state_type.h"
 #include "components/full_restore/app_launch_info.h"
 #include "components/full_restore/app_restore_data.h"
+#include "components/full_restore/window_info.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/window_open_disposition.h"
+#include "ui/gfx/geometry/rect.h"
 
 namespace full_restore {
 
@@ -38,6 +41,29 @@ const char kMimeType[] = "text/plain";
 const char kShareText1[] = "text1";
 const char kShareText2[] = "text2";
 
+const int32_t kActivationIndex1 = 100;
+const int32_t kActivationIndex2 = 101;
+const int32_t kActivationIndex3 = 102;
+
+const int32_t kDeskId1 = 1;
+const int32_t kDeskId2 = 2;
+const int32_t kDeskId3 = 3;
+
+const gfx::Rect kRestoreBounds1(10, 20, 110, 120);
+const gfx::Rect kRestoreBounds2(30, 40, 130, 140);
+const gfx::Rect kRestoreBounds3(50, 60, 150, 160);
+
+const gfx::Rect kCurrentBounds1(11, 21, 111, 121);
+const gfx::Rect kCurrentBounds2(31, 41, 131, 141);
+const gfx::Rect kCurrentBounds3(51, 61, 151, 161);
+
+const chromeos::WindowStateType kWindowStateType1 =
+    chromeos::WindowStateType::kMaximized;
+const chromeos::WindowStateType kWindowStateType2 =
+    chromeos::WindowStateType::kInactive;
+const chromeos::WindowStateType kWindowStateType3 =
+    chromeos::WindowStateType::kFullscreen;
+
 }  // namespace
 
 // Unit tests for restore data.
@@ -87,12 +113,44 @@ class RestoreDataTest : public testing::Test {
     restore_data().AddAppLaunchInfo(std::move(app_launch_info3));
   }
 
+  void ModifyWindowInfos() {
+    WindowInfo window_info1;
+    window_info1.activation_index = kActivationIndex1;
+    window_info1.desk_id = kDeskId1;
+    window_info1.restore_bounds = kRestoreBounds1;
+    window_info1.current_bounds = kCurrentBounds1;
+    window_info1.window_state_type = kWindowStateType1;
+
+    WindowInfo window_info2;
+    window_info2.activation_index = kActivationIndex2;
+    window_info2.desk_id = kDeskId2;
+    window_info2.restore_bounds = kRestoreBounds2;
+    window_info2.current_bounds = kCurrentBounds2;
+    window_info2.window_state_type = kWindowStateType2;
+
+    WindowInfo window_info3;
+    window_info3.activation_index = kActivationIndex3;
+    window_info3.desk_id = kDeskId3;
+    window_info3.restore_bounds = kRestoreBounds3;
+    window_info3.current_bounds = kCurrentBounds3;
+    window_info3.window_state_type = kWindowStateType3;
+
+    restore_data().ModifyWindowInfo(kAppId1, kId1, window_info1);
+    restore_data().ModifyWindowInfo(kAppId1, kId2, window_info2);
+    restore_data().ModifyWindowInfo(kAppId2, kId3, window_info3);
+  }
+
   void VerifyAppRestoreData(const std::unique_ptr<AppRestoreData>& data,
                             apps::mojom::LaunchContainer container,
                             WindowOpenDisposition disposition,
                             int64_t display_id,
                             std::vector<base::FilePath> file_paths,
-                            apps::mojom::IntentPtr intent) {
+                            apps::mojom::IntentPtr intent,
+                            int32_t activation_index,
+                            int32_t desk_id,
+                            const gfx::Rect& restore_bounds,
+                            const gfx::Rect& current_bounds,
+                            chromeos::WindowStateType window_state_type) {
     EXPECT_TRUE(data->container.has_value());
     EXPECT_EQ(static_cast<int>(container), data->container.value());
 
@@ -111,6 +169,21 @@ class RestoreDataTest : public testing::Test {
     EXPECT_EQ(intent->action, data->intent.value()->action);
     EXPECT_EQ(intent->mime_type, data->intent.value()->mime_type);
     EXPECT_EQ(intent->share_text, data->intent.value()->share_text);
+
+    EXPECT_TRUE(data->activation_index.has_value());
+    EXPECT_EQ(activation_index, data->activation_index.value());
+
+    EXPECT_TRUE(data->desk_id.has_value());
+    EXPECT_EQ(desk_id, data->desk_id.value());
+
+    EXPECT_TRUE(data->restore_bounds.has_value());
+    EXPECT_EQ(restore_bounds, data->restore_bounds.value());
+
+    EXPECT_TRUE(data->current_bounds.has_value());
+    EXPECT_EQ(current_bounds, data->current_bounds.value());
+
+    EXPECT_TRUE(data->window_state_type.has_value());
+    EXPECT_EQ(window_state_type, data->window_state_type.value());
   }
 
   void VerifyRestoreData(const RestoreData& restore_data) {
@@ -131,7 +204,9 @@ class RestoreDataTest : public testing::Test {
         WindowOpenDisposition::NEW_WINDOW, kDisplayId1,
         std::vector<base::FilePath>{base::FilePath(kFilePath1),
                                     base::FilePath(kFilePath2)},
-        CreateIntent(kIntentActionSend, kMimeType, kShareText1));
+        CreateIntent(kIntentActionSend, kMimeType, kShareText1),
+        kActivationIndex1, kDeskId1, kRestoreBounds1, kCurrentBounds1,
+        kWindowStateType1);
 
     const auto app_restore_data_it2 = launch_list_it1->second.find(kId2);
     EXPECT_TRUE(app_restore_data_it2 != launch_list_it1->second.end());
@@ -140,7 +215,9 @@ class RestoreDataTest : public testing::Test {
         apps::mojom::LaunchContainer::kLaunchContainerTab,
         WindowOpenDisposition::NEW_FOREGROUND_TAB, kDisplayId2,
         std::vector<base::FilePath>{base::FilePath(kFilePath2)},
-        CreateIntent(kIntentActionView, kMimeType, kShareText2));
+        CreateIntent(kIntentActionView, kMimeType, kShareText2),
+        kActivationIndex2, kDeskId2, kRestoreBounds2, kCurrentBounds2,
+        kWindowStateType2);
 
     // Verify for |kAppId2|
     const auto launch_list_it2 =
@@ -154,7 +231,9 @@ class RestoreDataTest : public testing::Test {
         apps::mojom::LaunchContainer::kLaunchContainerNone,
         WindowOpenDisposition::NEW_POPUP, kDisplayId2,
         std::vector<base::FilePath>{base::FilePath(kFilePath1)},
-        CreateIntent(kIntentActionView, kMimeType, kShareText1));
+        CreateIntent(kIntentActionView, kMimeType, kShareText1),
+        kActivationIndex3, kDeskId3, kRestoreBounds3, kCurrentBounds3,
+        kWindowStateType3);
   }
 
   RestoreData& restore_data() { return restore_data_; }
@@ -179,12 +258,13 @@ TEST_F(RestoreDataTest, AddNullAppLaunchInfo) {
 
 TEST_F(RestoreDataTest, AddAppLaunchInfos) {
   AddAppLaunchInfos();
-
+  ModifyWindowInfos();
   VerifyRestoreData(restore_data());
 }
 
 TEST_F(RestoreDataTest, Convert) {
   AddAppLaunchInfos();
+  ModifyWindowInfos();
   std::unique_ptr<base::Value> value =
       std::make_unique<base::Value>(restore_data().ConvertToValue());
   std::unique_ptr<RestoreData> restore_data =

components/full_restore/window_info.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_FULL_RESTORE_WINDOW_INFO_H_
 
 #include "base/optional.h"
+#include "chromeos/ui/base/window_state_type.h"
 #include "ui/aura/window.h"
 #include "ui/gfx/geometry/rect.h"
 
@@ -36,7 +37,7 @@ struct COMPONENT_EXPORT(FULL_RESTORE) WindowInfo {
   base::Optional<gfx::Rect> current_bounds;
 
   // Window state, minimized, maximized, inactive, etc.
-  base::Optional<int32_t> window_state_type;
+  base::Optional<chromeos::WindowStateType> window_state_type;
 };
 
 }  // namespace full_restore