Show notifications on low disk space.

chrome/browser/chromeos/ui/low_disk_notification.cc

+// 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.
+
+#include "chrome/browser/chromeos/ui/low_disk_notification.h"
+
+#include <stdint.h>
+
+#include "ash/common/system/system_notifier.h"
+#include "base/bind.h"
+#include "base/macros.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/grit/generated_resources.h"
+#include "chromeos/dbus/cryptohome_client.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/chromeos/resources/grit/ui_chromeos_resources.h"
+#include "ui/message_center/message_center.h"
+#include "ui/message_center/notification.h"
+#include "ui/message_center/notification_types.h"
+#include "ui/message_center/notifier_settings.h"
+
+namespace {
+const char kLowDiskId[] = "low_disk";
+const uint64_t kNotificationThreshold = 1 << 30;          // 1GB
+const uint64_t kNotificationSevereThreshold = 512 << 20;  // 512MB
+const base::TimeDelta kNotificationInterval = base::TimeDelta::FromMinutes(2);

[yoshiki, 2016/06/23 08:00:45]

constexpr is better than const.

[dspaid, 2016/06/27 05:40:04]

Done.

+}  // namespace
+
+namespace chromeos {
+
+LowDiskNotification::LowDiskNotification(CryptohomeClient* cryptohome_client)
+    : cryptohome_client_(cryptohome_client) {

[yoshiki, 2016/06/23 08:00:45]

I think it's better to use DBusThreadManager::Get()->GetCryptohomeClient()
directly, instead of passing it from the caller.

[dspaid, 2016/06/27 05:40:04]

Done.

+  if (cryptohome_client_) {

[yoshiki, 2016/06/23 08:00:45]

cryptohome_client_ is necessary for this class, so should you add
DCHECK(cryptohome_client_) instead of if-clause? 

[dspaid, 2016/06/27 05:40:04]

Done.

+    cryptohome_client_->SetLowDiskSpaceHandler(base::Bind(
+        &LowDiskNotification::OnLowDiskSpace, base::Unretained(this)));

[yoshiki, 2016/06/23 08:00:45]

You might need to close the notification when the disk gets enough space back?

[dspaid, 2016/06/27 05:40:04]

We don't actually have this information.  Notifications are sent when there is
not enough space, but there's no notification for having enough space.

+  }
+}
+
+LowDiskNotification::~LowDiskNotification() {

[yoshiki, 2016/06/23 08:00:45]

Please DCHECK(DBusThreadManager::Get()->GetCryptohomeClient()) to prevent them
from being destructed in wrong order.

[dspaid, 2016/06/27 05:40:04]

Done.

+  if (cryptohome_client_)
+    cryptohome_client_->ResetLowDiskSpaceHandler();
+}
+
+void LowDiskNotification::OnLowDiskSpace(uint64_t free_disk_bytes) {
+  LOG(ERROR) << "LowDiskNotification received";
+  Severity severity = GetSeverity(free_disk_bytes);
+  base::Time now = base::Time::Now();
+  if (severity > last_notification_severity_ ||

[yoshiki, 2016/06/23 08:00:45]

Shouldn't we update the notification content when the severity is changed down
to MEDIUM from HIGH?

[dspaid, 2016/06/27 05:40:04]

Done.

+      now - last_notification_time_ > kNotificationInterval) {
+    g_browser_process->message_center()->AddNotification(

[yoshiki, 2016/06/23 08:00:45]

You shouldn't pass nullptr to AddNotification.

[dspaid, 2016/06/27 05:40:04]

Done.

+        CreateNotification(severity));
+    last_notification_time_ = now;
+    last_notification_severity_ = severity;
+  }
+}
+
+std::unique_ptr<message_center::Notification>
+LowDiskNotification::CreateNotification(Severity severity) {
+  std::unique_ptr<message_center::Notification> notification;
+  switch (severity) {
+    case MEDIUM:
+      notification = message_center::Notification::CreateSystemNotification(
+          kLowDiskId,
+          l10n_util::GetStringUTF16(IDS_LOW_DISK_NOTIFICATION_TITLE),

[yoshiki, 2016/06/23 08:00:45]

Did you forget to add the string grd file to this patchset?

[dspaid, 2016/06/27 05:40:04]

They were in a separate change which has now been submitted.
https://codereview.chromium.org/2090913002

+          l10n_util::GetStringUTF16(IDS_LOW_DISK_NOTIFICATION_MESSAGE),
+          gfx::Image(ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+              IDR_DISK_SPACE_NOTIFICATION_LOW)),
+          ash::system_notifier::kNotifierDisk, base::Closure());
+      break;
+    case HIGH:
+      notification = message_center::Notification::CreateSystemNotification(

[yoshiki, 2016/06/23 08:00:45]

Parhaps, pinned notification is better?
https://cs.chromium.org/chromium/src/ui/message_center/notification.h?rcl=0&l...

fyi, pinned notification is a special type of notification which can't be
removed from the message center by user.

[dspaid, 2016/06/27 05:40:04]

Done.

+          kLowDiskId,
+          l10n_util::GetStringUTF16(IDS_CRITICALLY_LOW_DISK_NOTIFICATION_TITLE),
+          l10n_util::GetStringUTF16(
+              IDS_CRITICALLY_LOW_DISK_NOTIFICATION_MESSAGE),
+          gfx::Image(ui::ResourceBundle::GetSharedInstance().GetImageNamed(
+              IDR_DISK_SPACE_NOTIFICATION_CRITICAL)),
+          ash::system_notifier::kNotifierDisk, base::Closure());
+      break;
+    default:
+      return nullptr;
+  }
+  std::vector<message_center::ButtonInfo> buttons;
+  buttons.push_back(message_center::ButtonInfo(
+      l10n_util::GetStringUTF16(IDS_LOW_DISK_NOTIFICATION_BUTTON)));

[yoshiki, 2016/06/23 08:00:45]

Don't you add a click handler for this button?

[dspaid, 2016/06/27 05:40:04]

Done.

+  notification->set_buttons(buttons);
+  return notification;
+}
+
+LowDiskNotification::Severity LowDiskNotification::GetSeverity(
+    uint64_t free_disk_bytes) {
+  if (free_disk_bytes < kNotificationSevereThreshold)
+    return Severity::HIGH;
+  if (free_disk_bytes < kNotificationThreshold)
+    return Severity::MEDIUM;
+  return Severity::NONE;
+}
+
+}  // namespace chromeos