Add system row on the storage management page

The main sections on the storage management page have been added.
This change adds a "System" row so that the sizes of the storage items
of the storage page (My files, browsing data, apps etc...) add up to the
amount of "In use" space displayed at the top of the page.

This change creates a SystemSizeCalculator class that keeps track of all
calculated sizes.

The "System" storage size is displayed as
- "Calculating..." while the size of any other item is still being
calculated.
- "Unknown" if the calculation of any other row fails.
- The difference between "In use" and the sum of the other storage
items otherwise.

If the calculation of a row fails, the corresponding size in the
calculation of "System" will be 0.
In other words, when the calculation of a row fails, its size is
reflected in the System section.

Test:
unit_tests --gtest_filter="*StorageHandlerTest.SystemSize"
browser_tests --gtest_filter="*OSSettingsDevicePageTest.StorageTest"

Bug: 733192
Change-Id: Id3b82b10a2b94ed32daba085f8dce719d8272fe5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2060445
Commit-Queue: Jeremie Boulic <jboulic@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746569}

chrome/app/settings_strings.grdp

@@ -3052,6 +3052,9 @@
     <message name="IDS_SETTINGS_STORAGE_ITEM_AVAILABLE" desc="In Device Settings > Storage, label for the available storage size of Chrome OS internal storage.">
       Available
     </message>
+    <message name="IDS_SETTINGS_STORAGE_ITEM_SYSTEM" desc="In Device Settings > Storage, label for the total system size, difference between the size of the filesystem and the size of all items listed on the storage page.">
+      System
+    </message>
     <message name="IDS_SETTINGS_STORAGE_ITEM_MY_FILES" desc="In Device Settings > Storage, label for the size of My files root.">
       My files
     </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_STORAGE_ITEM_SYSTEM.png.sha1

+8a984b2c357750a862b81f4078e7c7b8a34fd52c
\ No newline at end of file

chrome/browser/resources/settings/chromeos/device_page/storage.html

@@ -227,6 +227,16 @@
           role-description="$i18n{subpageArrowRoleDescription}">
       </cr-link-row>
     </template>
+    <div class="settings-box two-line single-column stretch settings-box-text"
+        aria-describedby="systemSizeLabel"
+        aria-labelledby="systemSizeSubLabel">
+      <div id="systemSizeLabel" class="label" aria-hidden="true">
+        $i18n{storageItemSystem}
+      </div>
+      <div id="systemSizeSubLabel" class="secondary label" aria-hidden="true">
+        $i18n{storageSizeComputing}
+      </div>
+    </div>
     <template is="dom-if" if="[[androidEnabled]]">
       <cr-link-row id="externalStoragePreferences" class="hr"
           on-click="onExternalStoragePreferencesTap_"

chrome/browser/resources/settings/chromeos/device_page/storage.js

@@ -92,6 +92,9 @@ Polymer({
     this.addWebUIListener(
         'storage-crostini-size-changed',
         this.handleCrostiniSizeChanged_.bind(this));
+    this.addWebUIListener(
+        'storage-system-size-changed',
+        this.handleSystemSizeChanged_.bind(this));
     if (!this.isGuest_) {
       this.addWebUIListener(
           'storage-other-users-size-changed',
@@ -249,6 +252,14 @@ Polymer({
     this.$$('#otherUsersSize').subLabel = size;
   },
 
+  /**
+   * @param {string} size Formatted string representing the System size.
+   * @private
+   */
+  handleSystemSizeChanged_(size) {
+    this.$$('#systemSizeSubLabel').innerText = size;
+  },
+
   /**
    * @param {boolean} enabled True if Crostini is enabled.
    * @private

chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator.cc

@@ -49,9 +49,8 @@ void GetSizeStatBlocking(const base::FilePath& mount_path,
 
 }  // namespace
 
-SizeCalculator::SizeCalculator(const std::string& calculation_name) {
-  calculation_name_ = calculation_name;
-}
+SizeCalculator::SizeCalculator(const CalculationType& calculation_type)
+    : calculation_type_(calculation_type) {}
 
 SizeCalculator::~SizeCalculator() {}
 
@@ -75,13 +74,12 @@ void SizeCalculator::NotifySizeCalculated(
     const base::Optional<int64_t>& available_bytes) {
   calculating_ = false;
   for (SizeCalculator::Observer& observer : observers_) {
-    observer.OnSizeCalculated(calculation_name_, total_bytes, available_bytes);
+    observer.OnSizeCalculated(calculation_type_, total_bytes, available_bytes);
   }
 }
 
-SizeStatCalculator::SizeStatCalculator(const std::string& calculation_name,
-                                       Profile* profile)
-    : SizeCalculator(calculation_name), profile_(profile) {}
+SizeStatCalculator::SizeStatCalculator(Profile* profile)
+    : SizeCalculator(CalculationType::kInUse), profile_(profile) {}
 
 SizeStatCalculator::~SizeStatCalculator() = default;
 void SizeStatCalculator::PerformCalculation() {
@@ -105,10 +103,8 @@ void SizeStatCalculator::OnGetSizeStat(int64_t* total_bytes,
   NotifySizeCalculated(*total_bytes, *available_bytes);
 }
 
-MyFilesSizeCalculator::MyFilesSizeCalculator(
-    const std::string& calculation_name,
-    Profile* profile)
-    : SizeCalculator(calculation_name), profile_(profile) {}
+MyFilesSizeCalculator::MyFilesSizeCalculator(Profile* profile)
+    : SizeCalculator(CalculationType::kMyFiles), profile_(profile) {}
 
 MyFilesSizeCalculator::~MyFilesSizeCalculator() = default;
 
@@ -153,10 +149,8 @@ void MyFilesSizeCalculator::OnGetMyFilesSize(int64_t total_bytes) {
   NotifySizeCalculated(total_bytes);
 }
 
-BrowsingDataSizeCalculator::BrowsingDataSizeCalculator(
-    const std::string& calculation_name,
-    Profile* profile)
-    : SizeCalculator(calculation_name), profile_(profile) {}
+BrowsingDataSizeCalculator::BrowsingDataSizeCalculator(Profile* profile)
+    : SizeCalculator(CalculationType::kBrowsingData), profile_(profile) {}
 
 BrowsingDataSizeCalculator::~BrowsingDataSizeCalculator() = default;
 
@@ -222,9 +216,8 @@ void BrowsingDataSizeCalculator::OnGetBrowsingDataSize(bool is_site_data,
   }
 }
 
-AppsSizeCalculator::AppsSizeCalculator(const std::string& calculation_name,
-                                       Profile* profile)
-    : SizeCalculator(calculation_name), profile_(profile) {}
+AppsSizeCalculator::AppsSizeCalculator(Profile* profile)
+    : SizeCalculator(CalculationType::kAppsExtensions), profile_(profile) {}
 
 AppsSizeCalculator::~AppsSizeCalculator() {
   arc::ArcServiceManager::Get()
@@ -334,10 +327,8 @@ void AppsSizeCalculator::UpdateAppsAndExtensionsSize() {
   }
 }
 
-CrostiniSizeCalculator::CrostiniSizeCalculator(
-    const std::string& calculation_name,
-    Profile* profile)
-    : SizeCalculator(calculation_name), profile_(profile) {}
+CrostiniSizeCalculator::CrostiniSizeCalculator(Profile* profile)
+    : SizeCalculator(CalculationType::kCrostini), profile_(profile) {}
 
 CrostiniSizeCalculator::~CrostiniSizeCalculator() = default;
 
@@ -358,9 +349,8 @@ void CrostiniSizeCalculator::OnGetCrostiniSize(crostini::CrostiniResult result,
   NotifySizeCalculated(total_bytes);
 }
 
-OtherUsersSizeCalculator::OtherUsersSizeCalculator(
-    const std::string& calculation_name)
-    : SizeCalculator(calculation_name) {}
+OtherUsersSizeCalculator::OtherUsersSizeCalculator()
+    : SizeCalculator(CalculationType::kOtherUsers) {}
 
 OtherUsersSizeCalculator::~OtherUsersSizeCalculator() = default;
 

chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator.h

@@ -5,8 +5,11 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_CALCULATOR_SIZE_CALCULATOR_H_
 #define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_CALCULATOR_SIZE_CALCULATOR_H_
 
+#include <array>
 #include <bitset>
+#include <memory>
 #include <string>
+#include <vector>
 
 #include "base/files/file_util.h"
 #include "base/memory/weak_ptr.h"
@@ -26,20 +29,37 @@ namespace chromeos {
 namespace settings {
 namespace calculator {
 
-// Base class for storage item size calculation. SizeCalculator instances are
-// designed to notify observers about the calculated sizes.
+// Base class for the calculation of a specific storage item. Instances of this
+// class rely on their observers calling StartCalculation, and are designed to
+// notify observers about the calculated sizes.
 class SizeCalculator {
  public:
+  // Enumeration listing the items displayed on the storage page.
+  enum class CalculationType {
+    kInUse = 0,
+    kMyFiles,
+    kBrowsingData,
+    kAppsExtensions,
+    kCrostini,
+    kOtherUsers,
+    kLast = kOtherUsers,
+    kSystem,
+  };
+
   // Implement this interface to be notified about item size callbacks.
   class Observer : public base::CheckedObserver {
    public:
     virtual void OnSizeCalculated(
-        const std::string& calculation_name,
+        const CalculationType& item_id,
         int64_t total_bytes,
         const base::Optional<int64_t>& available_bytes) = 0;
   };
 
-  explicit SizeCalculator(const std::string& calculation_name);
+  // Total number of storage items.
+  static constexpr int kCalculationTypeCount =
+      static_cast<int>(CalculationType::kLast) + 1;
+
+  explicit SizeCalculator(const CalculationType& calculation_type);
   virtual ~SizeCalculator();
 
   // Starts the size calculation of a given storage item.
@@ -55,26 +75,25 @@ class SizeCalculator {
   // Performs the size calculation.
   virtual void PerformCalculation() = 0;
 
-  // Notify the StorageHandler about the calculated storage item size.
+  // Notify the StorageHandler about the calculated storage item size
   void NotifySizeCalculated(
       int64_t total_bytes,
       const base::Optional<int64_t>& available_bytes = base::nullopt);
 
-  // Observers being notified about storage items size changes.
-  base::ObserverList<SizeCalculator::Observer> observers_;
+  // Item id.
+  const CalculationType calculation_type_;
 
   // Flag indicating that fetch operations for storage size are ongoing.
   bool calculating_ = false;
-
-  // Name associated to the item size to calculate.
-  std::string calculation_name_;
+  // Observers being notified about storage items size changes.
+  base::ObserverList<SizeCalculator::Observer> observers_;
 };
 
 // Class handling the interactions with the filesystem to get storage
 // statistics, using OnSizeStatCalculated to notify observers.
 class SizeStatCalculator : public SizeCalculator {
  public:
-  SizeStatCalculator(const std::string& calculation_name, Profile* profile);
+  explicit SizeStatCalculator(Profile* profile);
   ~SizeStatCalculator() override;
 
   SizeStatCalculator(const SizeStatCalculator&) = delete;
@@ -96,7 +115,7 @@ class SizeStatCalculator : public SizeCalculator {
 // files + Android Play files.
 class MyFilesSizeCalculator : public SizeCalculator {
  public:
-  MyFilesSizeCalculator(const std::string& calculation_name, Profile* profile);
+  explicit MyFilesSizeCalculator(Profile* profile);
   ~MyFilesSizeCalculator() override;
 
   MyFilesSizeCalculator(const MyFilesSizeCalculator&) = delete;
@@ -121,8 +140,7 @@ class MyFilesSizeCalculator : public SizeCalculator {
 // Class handling the calculation of browsing data and cache.
 class BrowsingDataSizeCalculator : public SizeCalculator {
  public:
-  BrowsingDataSizeCalculator(const std::string& calculation_name,
-                             Profile* profile);
+  explicit BrowsingDataSizeCalculator(Profile* profile);
   ~BrowsingDataSizeCalculator() override;
 
   BrowsingDataSizeCalculator(const BrowsingDataSizeCalculator&) = delete;
@@ -164,7 +182,7 @@ class AppsSizeCalculator
     : public SizeCalculator,
       public arc::ConnectionObserver<arc::mojom::StorageManagerInstance> {
  public:
-  AppsSizeCalculator(const std::string& calculation_name, Profile* profile);
+  explicit AppsSizeCalculator(Profile* profile);
   ~AppsSizeCalculator() override;
 
   AppsSizeCalculator(const AppsSizeCalculator&) = delete;
@@ -226,7 +244,7 @@ class AppsSizeCalculator
 // Class handling the calculation of crostini VM size.
 class CrostiniSizeCalculator : public SizeCalculator {
  public:
-  CrostiniSizeCalculator(const std::string& calculation_name, Profile* profile);
+  explicit CrostiniSizeCalculator(Profile* profile);
   ~CrostiniSizeCalculator() override;
 
   CrostiniSizeCalculator(const CrostiniSizeCalculator&) = delete;
@@ -247,7 +265,7 @@ class CrostiniSizeCalculator : public SizeCalculator {
 // Class handling the calculation of other users' cryptohomes.
 class OtherUsersSizeCalculator : public SizeCalculator {
  public:
-  explicit OtherUsersSizeCalculator(const std::string& calculation_name);
+  OtherUsersSizeCalculator();
   ~OtherUsersSizeCalculator() override;
 
   OtherUsersSizeCalculator(const OtherUsersSizeCalculator&) = delete;

chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator_test_api.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_CALCULATOR_SIZE_CALCULATOR_TEST_API_H_
 #define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_CALCULATOR_SIZE_CALCULATOR_TEST_API_H_
 
+#include <utility>
+
 #include "chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator.h"
 #include "chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h"
 

chrome/browser/ui/webui/settings/chromeos/device_storage_handler.cc

@@ -6,7 +6,9 @@
 
 #include <algorithm>
 #include <limits>
+#include <memory>
 #include <string>
+#include <utility>
 
 #include "chrome/browser/chromeos/arc/arc_util.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
@@ -24,22 +26,42 @@ using chromeos::disks::DiskMountManager;
 namespace chromeos {
 namespace settings {
 
-
 namespace {
 
 constexpr char kAndroidEnabled[] = "androidEnabled";
 
+const char* CalculationTypeToEventName(
+    calculator::SizeCalculator::CalculationType x) {
+  switch (x) {
+    case calculator::SizeCalculator::CalculationType::kSystem:
+      return "storage-system-size-changed";
+    case calculator::SizeCalculator::CalculationType::kInUse:
+      return "storage-size-stat-changed";
+    case calculator::SizeCalculator::CalculationType::kMyFiles:
+      return "storage-my-files-size-changed";
+    case calculator::SizeCalculator::CalculationType::kBrowsingData:
+      return "storage-browsing-data-size-changed";
+    case calculator::SizeCalculator::CalculationType::kAppsExtensions:
+      return "storage-apps-size-changed";
+    case calculator::SizeCalculator::CalculationType::kCrostini:
+      return "storage-crostini-size-changed";
+    case calculator::SizeCalculator::CalculationType::kOtherUsers:
+      return "storage-other-users-size-changed";
+  }
+  NOTREACHED();
+  return "";
+}
+
 }  // namespace
 
 StorageHandler::StorageHandler(Profile* profile,
                                content::WebUIDataSource* html_source)
-    : size_stat_calculator_("storage-size-stat-changed", profile),
-      my_files_size_calculator_("storage-my-files-size-changed", profile),
-      browsing_data_size_calculator_("storage-browsing-data-size-changed",
-                                     profile),
-      apps_size_calculator_("storage-apps-size-changed", profile),
-      crostini_size_calculator_("storage-crostini-size-changed", profile),
-      other_users_size_calculator_("storage-other-users-size-changed"),
+    : size_stat_calculator_(profile),
+      my_files_size_calculator_(profile),
+      browsing_data_size_calculator_(profile),
+      apps_size_calculator_(profile),
+      crostini_size_calculator_(profile),
+      other_users_size_calculator_(),
       profile_(profile),
       source_name_(html_source->GetSource()),
       arc_observer_(this),
@@ -213,13 +235,13 @@ void StorageHandler::OnMountEvent(
 }
 
 void StorageHandler::OnSizeCalculated(
-    const std::string& event_name,
+    const calculator::SizeCalculator::CalculationType& calculation_type,
     int64_t total_bytes,
     const base::Optional<int64_t>& available_bytes) {
   if (available_bytes) {
-    UpdateSizeStat(event_name, total_bytes, available_bytes.value());
+    UpdateSizeStat(calculation_type, total_bytes, available_bytes.value());
   } else {
-    UpdateStorageItem(event_name, total_bytes);
+    UpdateStorageItem(calculation_type, total_bytes);
   }
 }
 
@@ -236,8 +258,15 @@ void StorageHandler::StopObservingEvents() {
   other_users_size_calculator_.RemoveObserver(this);
 }
 
-void StorageHandler::UpdateStorageItem(const std::string& event_name,
-                                       int64_t total_bytes) {
+void StorageHandler::UpdateStorageItem(
+    const calculator::SizeCalculator::CalculationType& calculation_type,
+    int64_t total_bytes) {
+  // When the system size has been calculated, UpdateSystemSize calls this
+  // method with the calculation type kSystem. This check prevents an infinite
+  // loop.
+  if (calculation_type != calculator::SizeCalculator::CalculationType::kSystem)
+    UpdateSystemSize(calculation_type, total_bytes);
+
   base::string16 message;
   if (total_bytes < 0) {
     message = l10n_util::GetStringUTF16(IDS_SETTINGS_STORAGE_SIZE_UNKNOWN);
@@ -245,21 +274,26 @@ void StorageHandler::UpdateStorageItem(const std::string& event_name,
     message = ui::FormatBytes(total_bytes);
   }
 
-  if (event_name == "storage-other-users-size-changed") {
+  if (calculation_type ==
+      calculator::SizeCalculator::CalculationType::kOtherUsers) {
     bool no_other_users = (total_bytes == 0);
-    FireWebUIListener(event_name, base::Value(message),
-                      base::Value(no_other_users));
+    FireWebUIListener(CalculationTypeToEventName(calculation_type),
+                      base::Value(message), base::Value(no_other_users));
   } else {
-    FireWebUIListener(event_name, base::Value(message));
+    FireWebUIListener(CalculationTypeToEventName(calculation_type),
+                      base::Value(message));
   }
 }
 
-void StorageHandler::UpdateSizeStat(const std::string& event_name,
-                                    int64_t total_bytes,
-                                    int64_t available_bytes) {
+void StorageHandler::UpdateSizeStat(
+    const calculator::SizeCalculator::CalculationType& calculation_type,
+    int64_t total_bytes,
+    int64_t available_bytes) {
   int64_t rounded_total_bytes = RoundByteSize(total_bytes);
   int64_t in_use_total_bytes_ = rounded_total_bytes - available_bytes;
 
+  UpdateSystemSize(calculation_type, in_use_total_bytes_);
+
   base::DictionaryValue size_stat;
   size_stat.SetString("availableSize", ui::FormatBytes(available_bytes));
   size_stat.SetString("usedSize", ui::FormatBytes(in_use_total_bytes_));
@@ -274,7 +308,38 @@ void StorageHandler::UpdateSizeStat(const std::string& event_name,
     storage_space_state = static_cast<int>(StorageSpaceState::kStorageSpaceLow);
   size_stat.SetInteger("spaceState", storage_space_state);
 
-  FireWebUIListener(event_name, size_stat);
+  FireWebUIListener(CalculationTypeToEventName(calculation_type), size_stat);
+}
+
+void StorageHandler::UpdateSystemSize(
+    const calculator::SizeCalculator::CalculationType& calculation_type,
+    int64_t total_bytes) {
+  const int item_index = static_cast<int>(calculation_type);
+  storage_items_total_bytes_[item_index] = total_bytes > 0 ? total_bytes : 0;
+  calculation_state_.set(item_index);
+
+  // Update system size. We only display the total system size when the size of
+  // all categories has been updated. If some size calculations are pending,
+  // return early and wait for all calculations to complete.
+  if (!calculation_state_.all())
+    return;
+
+  int64_t system_bytes = 0;
+  for (int i = 0; i < calculator::SizeCalculator::kCalculationTypeCount; ++i) {
+    int64_t total_bytes_for_current_item = storage_items_total_bytes_[i];
+    // If the storage is in use, add to the system's total storage.
+    if (i ==
+        static_cast<int>(calculator::SizeCalculator::CalculationType::kInUse)) {
+      system_bytes += total_bytes_for_current_item;
+      continue;
+    }
+    // Otherwise, this storage amount counts against the total storage
+    // amount.
+    system_bytes -= total_bytes_for_current_item;
+  }
+
+  OnSizeCalculated(calculator::SizeCalculator::CalculationType::kSystem,
+                   system_bytes);
 }
 
 bool StorageHandler::IsEligibleForAndroidStorage(std::string source_path) {

chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_DEVICE_STORAGE_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_DEVICE_STORAGE_HANDLER_H_
 
+#include <string>
+
 #include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
 #include "chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
@@ -62,9 +64,9 @@ class StorageHandler : public ::settings::SettingsPageUIHandler,
 
   // chromeos::settings::calculator::SizeCalculator::Observer:
   void OnSizeCalculated(
-      const std::string& event_name,
+      const calculator::SizeCalculator::CalculationType& calculation_type,
       int64_t total_bytes,
-      const base::Optional<int64_t>& available_bytes) override;
+      const base::Optional<int64_t>& available_bytes = base::nullopt) override;
 
   // Remove the handler from the list of observers of every observed instances.
   void StopObservingEvents();
@@ -83,10 +85,19 @@ class StorageHandler : public ::settings::SettingsPageUIHandler,
   void HandleUpdateExternalStorages(const base::ListValue* unused_args);
 
   // Update storage sizes on the UI.
-  void UpdateStorageItem(const std::string& event_name, int64_t total_bytes);
-  void UpdateSizeStat(const std::string& event_name,
-                      int64_t total_bytes,
-                      int64_t available_bytes);
+  void UpdateStorageItem(
+      const calculator::SizeCalculator::CalculationType& calculation_type,
+      int64_t total_bytes);
+  void UpdateSizeStat(
+      const calculator::SizeCalculator::CalculationType& calculation_type,
+      int64_t total_bytes,
+      int64_t available_bytes);
+
+  // Marks the size of |item| as calculated. When all storage items have been
+  // calculated, then "System" size can be calculated.
+  void UpdateSystemSize(
+      const calculator::SizeCalculator::CalculationType& calculation_type,
+      int64_t total_bytes);
 
   // Updates list of external storages.
   void UpdateExternalStorages();
@@ -103,6 +114,14 @@ class StorageHandler : public ::settings::SettingsPageUIHandler,
   calculator::CrostiniSizeCalculator crostini_size_calculator_;
   calculator::OtherUsersSizeCalculator other_users_size_calculator_;
 
+  // Controls if the size of each storage item has been calculated.
+  std::bitset<calculator::SizeCalculator::kCalculationTypeCount>
+      calculation_state_;
+
+  // Keeps track of the size of each storage item.
+  int64_t storage_items_total_bytes_
+      [calculator::SizeCalculator::kCalculationTypeCount] = {0};
+
   Profile* const profile_;
   const std::string source_name_;
   ScopedObserver<arc::ArcSessionManager, arc::ArcSessionManager::Observer>

chrome/browser/ui/webui/settings/chromeos/device_storage_handler_unittest.cc

@@ -2,8 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h"
-#include "chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator_test_api.h"
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
 
 #include "base/files/file.h"
 #include "base/files/file_util.h"
@@ -15,6 +17,8 @@
 #include "chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h"
 #include "chrome/browser/chromeos/file_manager/path_util.h"
 #include "chrome/browser/chromeos/scoped_set_running_on_chromeos_for_testing.h"
+#include "chrome/browser/ui/webui/settings/chromeos/calculator/size_calculator_test_api.h"
+#include "chrome/browser/ui/webui/settings/chromeos/device_storage_handler.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -78,26 +82,20 @@ class StorageHandlerTest : public testing::Test {
 
     // Initialize tests APIs.
     size_stat_test_api_ = std::make_unique<calculator::SizeStatTestAPI>(
-        handler_.get(), new calculator::SizeStatCalculator(
-                            "storage-size-stat-changed", profile_));
+        handler_.get(), new calculator::SizeStatCalculator(profile_));
     my_files_size_test_api_ = std::make_unique<calculator::MyFilesSizeTestAPI>(
-        handler_.get(), new calculator::MyFilesSizeCalculator(
-                            "storage-my-files-size-changed", profile_));
+        handler_.get(), new calculator::MyFilesSizeCalculator(profile_));
     browsing_data_size_test_api_ =
         std::make_unique<calculator::BrowsingDataSizeTestAPI>(
             handler_.get(),
-            new calculator::BrowsingDataSizeCalculator(
-                "storage-browsing-data-size-changed", profile_));
+            new calculator::BrowsingDataSizeCalculator(profile_));
     apps_size_test_api_ = std::make_unique<calculator::AppsSizeTestAPI>(
-        handler_.get(), new calculator::AppsSizeCalculator(
-                            "storage-apps-size-changed", profile_));
+        handler_.get(), new calculator::AppsSizeCalculator(profile_));
     crostini_size_test_api_ = std::make_unique<calculator::CrostiniSizeTestAPI>(
-        handler_.get(), new calculator::CrostiniSizeCalculator(
-                            "storage-crostini-size-changed", profile_));
+        handler_.get(), new calculator::CrostiniSizeCalculator(profile_));
     other_users_size_test_api_ =
         std::make_unique<calculator::OtherUsersSizeTestAPI>(
-            handler_.get(), new calculator::OtherUsersSizeCalculator(
-                                "storage-other-users-size-changed"));
+            handler_.get(), new calculator::OtherUsersSizeCalculator());
 
     // Create and register My files directory.
     // By emulating chromeos running, GetMyFilesFolderForProfile will return the
@@ -392,6 +390,126 @@ TEST_F(StorageHandlerTest, AppsExtensionsSize) {
   EXPECT_EQ("401 KB", callback->GetString());
 }
 
+TEST_F(StorageHandlerTest, SystemSize) {
+  // The "System" row on the storage page displays the difference between the
+  // total amount of used space and the sum of the sizes of the different
+  // storage items of the storage page (My files, Browsing data, apps etc...)
+  // This test simulates callbacks from each one of these storage items; the
+  // calculation of the "System" size should only happen when all of the other
+  // storage items have been calculated.
+  const int64_t KB = 1024;
+  const int64_t MB = 1024 * KB;
+  const int64_t GB = 1024 * MB;
+  const int64_t TB = 1024 * GB;
+
+  // Simulate size stat callback.
+  int64_t total_size = TB;
+  int64_t available_size = 100 * GB;
+  size_stat_test_api_->SimulateOnGetSizeStat(&total_size, &available_size);
+  const base::Value* callback =
+      GetWebUICallbackMessage("storage-size-stat-changed");
+  ASSERT_TRUE(callback) << "No 'storage-size-stat-changed' callback";
+  EXPECT_EQ("100 GB", callback->FindKey("availableSize")->GetString());
+  EXPECT_EQ("924 GB", callback->FindKey("usedSize")->GetString());
+  // Expect no system size callback until every other item has been updated.
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+
+  // Simulate my files size callback.
+  my_files_size_test_api_->SimulateOnGetTotalBytes(400 * GB);
+  callback = GetWebUICallbackMessage("storage-my-files-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-my-files-size-changed' callback";
+  EXPECT_EQ("400 GB", callback->GetString());
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+
+  // Simulate browsing data callbacks. Has to be called with
+  // both |is_data_site| = true and false.
+  browsing_data_size_test_api_->SimulateOnGetBrowsingDataSize(
+      true /* is_site_data */, 10 * GB);
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-browsing-data-size-changed"));
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+  browsing_data_size_test_api_->SimulateOnGetBrowsingDataSize(
+      false /* is_site_data */, 14 * GB);
+  callback = GetWebUICallbackMessage("storage-browsing-data-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-browsing-data-size-changed' callback";
+  EXPECT_EQ("24.0 GB", callback->GetString());
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+
+  // Simulate apps and extensions size callbacks.
+  apps_size_test_api_->SimulateOnGetAppsSize(29 * GB);
+  apps_size_test_api_->SimulateOnGetAndroidAppsSize(false, 0, 0, 0);
+  callback = GetWebUICallbackMessage("storage-apps-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-apps-size-changed' callback";
+  EXPECT_EQ("29.0 GB", callback->GetString());
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+  apps_size_test_api_->SimulateOnGetAndroidAppsSize(
+      true /* succeeded */, 724 * MB, 100 * MB, 200 * MB);
+  callback = GetWebUICallbackMessage("storage-apps-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-apps-size-changed' callback";
+  EXPECT_EQ("30.0 GB", callback->GetString());
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+
+  // Simulate crostini size callback.
+  crostini_size_test_api_->SimulateOnGetCrostiniSize(70 * GB);
+  callback = GetWebUICallbackMessage("storage-crostini-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-crostini-size-changed' callback";
+  EXPECT_EQ("70.0 GB", callback->GetString());
+  ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+
+  // Simulate other users size callback. No callback message until the sizes of
+  // every users is calculated.
+  std::vector<int64_t> other_user_sizes =
+      std::vector<int64_t>{200 * GB, 50 * GB, 50 * GB};
+  other_users_size_test_api_->InitializeOtherUserSize(other_user_sizes.size());
+  for (std::size_t i = 0; i < other_user_sizes.size(); i++) {
+    cryptohome::BaseReply result;
+    result.set_error(cryptohome::CRYPTOHOME_ERROR_NOT_SET);
+    cryptohome::GetAccountDiskUsageReply* usage_reply =
+        result.MutableExtension(cryptohome::GetAccountDiskUsageReply::reply);
+    usage_reply->set_size(other_user_sizes[i]);
+    base::Optional<cryptohome::BaseReply> reply = std::move(result);
+    other_users_size_test_api_->SimulateOnGetOtherUserSize(reply);
+    if (i < other_user_sizes.size() - 1) {
+      ASSERT_FALSE(GetWebUICallbackMessage("storage-other-users-size-changed"));
+      ASSERT_FALSE(GetWebUICallbackMessage("storage-system-size-changed"));
+    } else {
+      // When the size of the last user's cryptohome is calculated, we expect a
+      // callback with the "Other users" size.
+      callback = GetWebUICallbackMessage("storage-other-users-size-changed");
+      ASSERT_TRUE(callback) << "No 'storage-other-users-size-changed' callback";
+      EXPECT_EQ("300 GB", callback->GetString());
+      // Every item size has been calculated, system size should also be
+      // updated.
+      callback = GetWebUICallbackMessage("storage-system-size-changed");
+      ASSERT_TRUE(callback) << "No 'storage-system-size-changed' callback";
+      EXPECT_EQ("100 GB", callback->GetString());
+    }
+  }
+
+  // If there's an error while calculating the size of browsing data, the size
+  // of browsing data and system should be displayed as "Unknown".
+  browsing_data_size_test_api_->SimulateOnGetBrowsingDataSize(
+      true /* is_site_data */, -1);
+  callback = GetWebUICallbackMessage("storage-browsing-data-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-browsing-data-size-changed' callback";
+  EXPECT_EQ("Unknown", callback->GetString());
+  // The missing 24.0 GB of browsing data should be reflected in the system
+  // section instead. We expect the displayed size to be 100 + 24 GB.
+  callback = GetWebUICallbackMessage("storage-system-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-system-size-changed' callback";
+  EXPECT_EQ("124 GB", callback->GetString());
+
+  // No error while recalculating browsing data size, the UI should be updated
+  // with the right sizes.
+  browsing_data_size_test_api_->SimulateOnGetBrowsingDataSize(
+      true /* is_site_data */, 10 * GB);
+  callback = GetWebUICallbackMessage("storage-browsing-data-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-browsing-data-size-changed' callback";
+  EXPECT_EQ("24.0 GB", callback->GetString());
+  callback = GetWebUICallbackMessage("storage-system-size-changed");
+  ASSERT_TRUE(callback) << "No 'storage-system-size-changed' callback";
+  EXPECT_EQ("100 GB", callback->GetString());
+}
+
 }  // namespace
 
 }  // namespace settings

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider.cc

@@ -1073,6 +1073,7 @@ void AddDeviceStorageStrings(content::WebUIDataSource* html_source) {
       {"storageTitle", IDS_SETTINGS_STORAGE_TITLE},
       {"storageItemInUse", IDS_SETTINGS_STORAGE_ITEM_IN_USE},
       {"storageItemAvailable", IDS_SETTINGS_STORAGE_ITEM_AVAILABLE},
+      {"storageItemSystem", IDS_SETTINGS_STORAGE_ITEM_SYSTEM},
       {"storageItemMyFiles", IDS_SETTINGS_STORAGE_ITEM_MY_FILES},
       {"storageItemBrowsingData", IDS_SETTINGS_STORAGE_ITEM_BROWSING_DATA},
       {"storageItemApps", IDS_SETTINGS_STORAGE_ITEM_APPS},

chrome/test/data/webui/settings/chromeos/device_page_tests.js

@@ -1924,6 +1924,17 @@ cr.define('device_page_tests', function() {
                 .innerText);
       });
 
+      test('system size', async function() {
+        assertEquals('System', storagePage.$$('#systemSizeLabel').innerText);
+        assertEquals(
+            'Calculating…', storagePage.$$('#systemSizeSubLabel').innerText);
+
+        // Send system size callback.
+        cr.webUIListenerCallback('storage-system-size-changed', '8.4 GB');
+        Polymer.dom.flush();
+        assertEquals('8.4 GB', storagePage.$$('#systemSizeSubLabel').innerText);
+      });
+
       test('apps extensions size', async function() {
         assertEquals(
             'Apps and extensions', getStorageItemLabelFromId('appsSize'));