Port storage manager to MD settings.

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

+// Copyright 2016 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.
+
+/**
+ * @fileoverview
+ * 'settings-storage' is the settings subpage for storage settings.
+ */
+
+/**
+ * Enumeration for device state about remaining space.
+ * These values must be kept in sync with
+ * StorageManagerHandler::StorageSpaceState in C++ code.
+ * @enum {number}
+ */
+var StorageSpaceState = {

[michaelpg, 2016/10/01 01:41:05]

namespace these in 'settings' via cr.define; see
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/site_s...

[fukino, 2016/10/03 18:40:41]

As far as I tried, an enum defined in a namespace is reported as Unknown type.

cr.define('settings', function() {
  /** @enum {string} */
  var Foo = {A: 'a'};

  /** @constructor */
  function Bar() {}

  return {
    Foo: Foo,
    Bar: Bar,
  };
});

/** @type {settings.Foo} */
var foo; // This doesn't compile, saying "ERROR - Bad type annotation. Unknown
type settings.Foo"

/** @type {settings.Bar} */
var bar; // This compiles.

Maybe there is an issue in ChromePass (or I missed something)?

[michaelpg, 2016/10/03 23:44:44]

You're right, I forgot about that. I think it's a problem with Closure's type
inference system.

If you want, you can use cr.exportPath('settings'); and then define these as
settings.Foo = {} outside of the cr.define:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/people...

But that's optional, we just use "var Foo" like you have it here in several
places because of the closure bug.

[fukino, 2016/10/04 09:03:53]

Thanks!
I used cr.exportPath('settings') to keep them in settings namespace.

+  STORAGE_SPACE_NORMAL: 0,

[michaelpg, 2016/10/01 01:41:05]

why not just NORMAL, LOW, CRITICALLY_LOW?

[fukino, 2016/10/03 18:40:41]

Done.

+  STORAGE_SPACE_LOW: 1,
+  STORAGE_SPACE_CRITICALLY_LOW: 2
+};
+
+/**
+ * @typedef {{
+ *   totalSize: string,
+ *   availableSize: string,
+ *   usedSize: string,
+ *   usedRatio: number,
+ *   spaceState: StorageSpaceState,
+ * }}
+ */
+var StorageSizeStat;
+
+Polymer({
+  is: 'settings-storage',
+
+  behaviors: [settings.RouteObserverBehavior, WebUIListenerBehavior],
+
+  properties: {
+    /** @private {boolean} */

[michaelpg, 2016/10/01 01:41:05]

the Polymer pass for closure infers type names for simple types, so the
{boolean} isn't needed

[fukino, 2016/10/03 18:40:41]

Done.

+    driveEnabled_: {
+      type: Boolean,
+      value: false,
+    },
+
+    /** @private {boolean} */
+    androidEnabled_: {
+      type: Boolean,
+      value: false,
+    },
+
+    /** @private {number} */
+    usedRatio_: {
+      type: Number,
+      value: 0,
+    },
+
+    /** @private {number} */

[michaelpg, 2016/10/01 01:41:05]

here, though, you want to provide closure with *more* type information, since
it's an enum: @private {settings.StorageSpaceState}

[fukino, 2016/10/03 18:40:41]

Done.

+    spaceState_: {
+      type: Number,
+      value: StorageSpaceState.STORAGE_SPACE_NORMAL,
+    },
+  },
+
+  /**
+   * Timer ID for periodical update.

[michaelpg, 2016/10/01 01:41:05]

nit: "periodic" :-)

[fukino, 2016/10/03 18:40:41]

Oops! Done.

+   * @private {number}
+   */
+  updateTimerId_: -1,
+
+  /**
+   * True if the current route is STORAGE.
+   * @private {boolean}
+   */
+  visible_: false,

[michaelpg, 2016/10/01 01:41:05]

what is this used for?

[fukino, 2016/10/03 18:40:41]

I couldn't find how to know the previous route, so I add a variable |visible_|
to know the previous state about whether the current route was STORAGE or not.

However, thanks to your suggestion about the periodic timer, I don't need
|visible_| anymore, so I removed it.

+
+  /** @override */
+  ready: function() {
+    cr.addWebUIListener(
+        'storage-size-stat-changed',
+        this.handleSizeStatChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-downloads-size-changed',
+        this.handleDownloadsSizeChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-drive-cache-size-changed',
+        this.handleDriveCacheSizeChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-browsing-data-size-changed',
+        this.handleBrowsingDataSizeChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-android-size-changed',
+        this.handleAndroidSizeChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-other-users-size-changed',
+        this.handleOtherUsersSizeChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-drive-enabled-changed',
+        this.handleDriveEnabledChanged_.bind(this));
+    cr.addWebUIListener(
+        'storage-android-enabled-changed',
+        this.handleAndroidEnabledChanged_.bind(this));
+  },
+
+  /**
+   * Overridden from settings.RouteObserverBehavior.
+   * @protected
+   */
+  currentRouteChanged: function() {
+    if (settings.getCurrentRoute() == settings.Route.STORAGE) {
+      if (!this.visible_) {
+        this.visible_ = true;
+        this.onPageShown_();
+      }
+    } else {
+      if (this.visible_) {
+        this.visible_ = false;
+        this.onPageHidden_();
+      }
+    }
+  },
+
+  /** @private */
+  onPageShown_: function() {
+    // Updating storage information can be expensive (e.g. computing directory
+    // sizes recursively), so we delay this operation until the page is shown.
+    chrome.send('updateStorageInfo');
+    // We periodically update the storage usage while the overlay is visible.
+    this.startPeriodicalUpdate_();

[michaelpg, 2016/10/01 01:41:05]

same, "Periodic" (but "periodically" is correct. english is weird)

[fukino, 2016/10/03 18:40:41]

Done.

+  },
+
+  /** @private */
+  onPageHidden_: function() {
+    this.stopPeriodicalUpdate_();
+  },
+
+  /**
+   * Handler for tapping the "Downloads" item.
+   * @private
+   */
+  onDownloadsTap_: function() {
+    chrome.send('openDownloads');
+  },
+
+  /**
+   * Handler for tapping the "Offlien files" item.

[michaelpg, 2016/10/01 01:41:05]

Offline

[fukino, 2016/10/03 18:40:41]

Done.

+   * @private
+   */
+  onDriveCacheTap_: function() {
+    // TODO(fukino): Show a dialog of Drive cache.

[michaelpg, 2016/10/01 01:41:05]

should this TODO go in drive_cache_dialog?

[fukino, 2016/10/03 18:40:41]

I removed the TODO comment. (The dialog was already implemented.)

+    this.$.storageDriveCache.open();
+  },
+
+  /**
+   * Handler for tapping the "Browsing data" item.
+   * @private
+   */
+  onBrowsingDataTap_: function() {
+    settings.navigateTo(settings.Route.CLEAR_BROWSER_DATA);
+  },
+
+  /**
+   * Handler for tapping the "Android storage" item.
+   * @private
+   */
+  onAndroidTap_: function() {
+    chrome.send('openArcStorage');
+  },
+
+  /**
+   * Handler for tapping the "Other users" item.
+   * @private
+   */
+  onOtherUsersTap_: function() {
+    settings.navigateTo(settings.Route.ACCOUNTS);
+  },
+
+  /**
+   * @param {!StorageSizeStat} sizeStat
+   * @private
+   */
+  handleSizeStatChanged_: function(sizeStat) {
+    this.usedRatio_ = sizeStat.usedRatio;

[michaelpg, 2016/10/01 01:41:05]

So, why not just have a sizeStat_ property? You can then use
sizeStat_.usedRatio, etc. in the HTML and JS. And instead of setting
textContent, you can use data binding: [[sizeStat_.usedSize]], etc

[fukino, 2016/10/03 18:40:41]

Wow nice cleanup!
I totally forgot the binding to subproperties...

+    this.spaceState_ = sizeStat.spaceState;
+    this.$.inUseSize.textContent = sizeStat.usedSize;
+    this.$.availableSize.textContent = sizeStat.availableSize;
+    this.$.inUseLabelArea.style.width = (sizeStat.usedRatio * 100) + '%';
+    this.$.availableLabelArea.style.width =
+        ((1 - sizeStat.usedRatio) * 100) + '%';
+  },
+
+  /**
+   * @param {string} size Formatted string representing the size of Downloads.
+   * @private
+   */
+  handleDownloadsSizeChanged_: function(size) {
+    this.$.downloadsSize.textContent = size;
+  },
+
+  /**
+   * @param {string} size Formatted string representing the size of Offline
+   *     files.
+   * @private
+   */
+  handleDriveCacheSizeChanged_: function(size) {
+    this.$.driveCacheSize.textContent = size;
+  },
+
+  /**
+   * @param {string} size Formatted string representing the size of Browsing
+   *     data.
+   * @private
+   */
+  handleBrowsingDataSizeChanged_: function(size) {
+    this.$.browsingDataSize.textContent = size;
+  },
+
+  /**
+   * @param {string} size Formatted string representing the size of Android
+   *     storage.
+   * @private
+   */
+  handleAndroidSizeChanged_: function(size) {
+    this.$.androidSize.textContent = size;
+  },
+
+  /**
+   * @param {string} size Formatted string representing the size of Other users.
+   * @private
+   */
+  handleOtherUsersSizeChanged_: function(size) {
+    this.$.otherUsersSize.textContent = size;
+  },
+
+  /**
+   * @param {boolean} enabled True if Google Drive is enabled.
+   * @private
+   */
+  handleDriveEnabledChanged_: function(enabled) {
+    this.driveEnabled_ = enabled;
+  },
+
+  /**
+   * @param {boolean} enabled True if Android Play Store is enabled.
+   * @private
+   */
+  handleAndroidEnabledChanged_: function(enabled) {
+    this.androidEnabled_ = enabled;
+  },
+
+  /**
+   * Starts periodical update for storage usage.
+   * @private
+   */
+  startPeriodicalUpdate_: function() {
+    // We update the storage usage every 5 seconds.
+    if (this.updateTimerId_ == -1) {
+      this.updateTimerId_ = window.setInterval(function() {
+        chrome.send('updateStorageInfo');

[michaelpg, 2016/10/01 01:41:05]

I would feel better if we checked the route here, and stopped when it doesn't
match STORAGE anymore.

In particular, if this dialog gets detached, it stops observing
currentRouteChanged_:
https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/route....

Meaning I think this would continue getting called indefinitely. (We don't
currently detach hidden subpages, but we might start.)

Plus if we do the check here, we lower the risk of a bug elsewhere preventing us
from clearing the interval.

It also means we can just call window.clearInterval(this.updateTimerId_) right
here -- we don't need a separate function, checking updateTimerId_, etc. And we
can also simplify currentRouteChanged_ to just:

    if (settings.getCurrentRoute() == STORAGE)
      this.onPageShown_();

and schedule the interval there.

[fukino, 2016/10/03 18:40:41]

Oh, I didn't understand that the dialog can be detached. Thank you for pointing
it out.
I agree with your suggestion. Checking the route insider the timer function
sounds robust. I updated the code to do so.

+      }, 5000);
+    }
+  },
+
+  /**
+   * Stops periodical update for storage usage.
+   * @private
+   */
+  stopPeriodicalUpdate_: function() {
+    if (this.updateTimerId_ != -1) {
+      window.clearInterval(this.updateTimerId_);
+      this.updateTimerId_ = -1;
+    }
+  },
+
+  /**
+   * Returns true if the remaining space is low, but not critically low.
+   * @param {!StorageSpaceState} spaceState Status about the remaining space.
+   * @private
+   */
+  isSpaceLow_: function(spaceState) {
+    return spaceState == StorageSpaceState.STORAGE_SPACE_LOW;
+  },
+
+  /**
+   * Returns true if the remaining space is critically low.
+   * @param {!StorageSpaceState} spaceState Status about the remaining space.
+   * @private
+   */
+  isSpaceCriticallyLow_: function(spaceState) {
+    return spaceState == StorageSpaceState.STORAGE_SPACE_CRITICALLY_LOW;
+  },
+
+  /**
+   * Computes class name of the bar based on the remaining space size.
+   * @param {!StorageSpaceState} spaceState Status about the remaining space.
+   * @private
+   */
+  getBarClass_: function(spaceState) {
+    switch (spaceState) {
+      case StorageSpaceState.STORAGE_SPACE_LOW:
+        return 'space-low';
+      case StorageSpaceState.STORAGE_SPACE_CRITICALLY_LOW:
+        return 'space-critically-low';
+      default:
+        return '';
+    }
+  },
+});