Add ArcNotificationManager for new ARC notification

chrome/browser/chromeos/arc/notification/arc_notification_item.cc

+// Copyright 2015 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/arc/notification/arc_notification_item.h"
+
+#include "base/strings/string16.h"
+#include "base/strings/utf_string_conversions.h"
+#include "ui/gfx/image/image.h"
+#include "ui/message_center/notification.h"
+#include "ui/message_center/notification_types.h"
+#include "ui/message_center/notifier_settings.h"
+
+#if defined(USE_ASH)
+#include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
+#include "components/signin/core/account_id/account_id.h"
+#endif
+
+namespace {
+
+// static
+static const char* kNotificationIdPrefix = "ARC_NOTIFICATION_";
+
+class ArcNotificationDelegate : public message_center::NotificationDelegate {
+ public:
+  explicit ArcNotificationDelegate(
+      base::WeakPtr<chromeos::ArcNotificationItem> item)
+      : item_(item) {}
+
+  void Close(bool by_user) override {
+    if (item_)
+      item_->Close(by_user);
+  }
+
+  // Indicates all notifications have a click handler. This changes the mouse
+  // cursor on hover.
+  // TODO(yoshiki): Return the corerct value according to the content intent

[hidehiko, 2015/12/11 05:18:00]

nit: s/corerct/correct/

[yoshiki, 2015/12/14 15:19:29]

Done.

+  // and the flags.
+  bool HasClickedListener() override { return true; }
+
+  void Click() override {
+    if (item_)
+      item_->Click();
+  }
+
+ private:
+  // The destractor is private since this class is ref-counted.
+  ~ArcNotificationDelegate() override {}
+
+  base::WeakPtr<chromeos::ArcNotificationItem> item_;
+};
+
+}  // anonymous namespace
+
+namespace chromeos {
+
+ArcNotificationItem::ArcNotificationItem(
+    ArcNotificationManager* manager,
+    message_center::MessageCenter* message_center,
+    const arc::ArcNotificationData& data,
+    Profile* profile)
+    : manager_(manager),
+      message_center_(message_center),
+      profile_(profile),
+      weak_ptr_factory_(this) {
+  // This must be initialized after ArcBridgeService.
+  notification_key_ = data.key;

[hidehiko, 2015/12/11 05:18:00]

Please move them to the initializer list.

[yoshiki, 2015/12/14 15:19:29]

Done.

+  notification_id_ = kNotificationIdPrefix + notification_key_;
+
+  UpdateWithArcNotificationData(data);

[hidehiko, 2015/12/11 05:18:00]

IMHO, you can defer the responsibility to call this method to the caller.

[yoshiki, 2015/12/14 15:19:29]

Done.

+}
+
+void ArcNotificationItem::UpdateWithArcNotificationData(
+    const arc::ArcNotificationData& data) {
+  DCHECK(notification_key_ == data.key);
+
+  message_center::RichNotificationData rich_data;
+  message_center::NotificationType type =

[hidehiko, 2015/12/11 05:18:00]

I prefer move this to default:, as you set it in the "case
arc::ARC_NOTIFICATION_TYPE_PROGRESS".

[yoshiki, 2015/12/14 15:19:29]

I moved them into the switch block, since I removed the default.

+      message_center::NOTIFICATION_TYPE_SIMPLE;
+  switch (data.type) {
+    case arc::ARC_NOTIFICATION_TYPE_PROGRESS:
+      type = message_center::NOTIFICATION_TYPE_PROGRESS;
+      rich_data.timestamp = base::Time::UnixEpoch() +
+                            base::TimeDelta::FromMilliseconds(data.time);
+      rich_data.progress = std::floor(
+          static_cast<float>(data.progress_current) / data.progress_max * 100 +
+          0.5);  // round off

[hidehiko, 2015/12/11 05:18:00]

nit: std::round is your friend.

BTW, the values, |progress_current| and |progress_max|, can be trusted?
Otherwise, probably you may want to clamp with [0, 100] ?

[yoshiki, 2015/12/14 15:19:29]

Thank you for letting me know round(), done.

+      break;
+    default:
+      // TODO(yoshiki): Implement other types.
+      break;
+  }
+
+  message_center::NotifierId notifier_id(
+      message_center::NotifierId::SYSTEM_COMPONENT, notification_id_);
+#if defined(USE_ASH)
+  notifier_id.profile_id =
+      multi_user_util::GetAccountIdFromProfile(profile_).GetUserEmail();
+#endif
+
+  DCHECK(!data.title.is_null());
+  DCHECK(!data.message.is_null());
+  notification_.reset(new message_center::Notification(
+      type, notification_id_, base::UTF8ToUTF16(data.title.get()),
+      base::UTF8ToUTF16(data.message.get()),
+      gfx::Image(),              // Will be overriden by decODED IMAGE.
+      base::UTF8ToUTF16("arc"),  // display source
+      GURL(),                    // origin url
+      notifier_id, rich_data,
+      new ArcNotificationDelegate(weak_ptr_factory_.GetWeakPtr())));
+
+  DCHECK(!data.icon_data.is_null());
+  DCHECK(data.icon_data.size() > 0);
+  std::string icon_data_str(data.icon_data.storage().begin(),
+                            data.icon_data.storage().end());  // copy
+  ImageDecoder::Start(this, icon_data_str);  // copy in the method.
+}
+
+ArcNotificationItem::~ArcNotificationItem() {}
+
+void ArcNotificationItem::OnClosedFromAndroid() {
+  message_center_->RemoveNotification(notification_id_, true /* by_user */);
+}
+
+void ArcNotificationItem::Close(bool by_user) {
+  manager_->SendNotificationRemovedFromChrome(notification_key_);
+}
+
+void ArcNotificationItem::Click() {
+  manager_->SendNotificationClickedOnChrome(notification_key_);
+}
+
+void ArcNotificationItem::OnImageDecoded(const SkBitmap& bitmap) {
+  gfx::Image image = gfx::Image::CreateFrom1xBitmap(bitmap);
+  notification_->set_icon(image);
+
+  message_center_->AddNotification(make_scoped_ptr(

[hidehiko, 2015/12/11 05:18:00]

Looks race?

- UpdateWithArcNotificationData(...).
- Before decode is done OnClosedFromAndroid()/

Also,
- UpdateWithArcNotificationData(...).
- Before decode is done, UpdateWithArcNotificationData(...).

[yoshiki, 2015/12/14 15:19:29]

It shouldn't be occurred. Because this instance is free'd soon after
OnClosedFromAndroid(). But I agree that it's too complex to understand that.
Yes it should be race. I added some logic to prevent multiple decodes at the
same time. So if UpdateWithArcNotificationData(...) is called during decoding,
the updating is delayed.

+      new message_center::Notification(*notification_)));  // copy

[hidehiko, 2015/12/11 05:18:00]

You do not need a copy here, instead you can move the |notification_|.

[yoshiki, 2015/12/14 15:19:29]

I thought I wanted to have the data stored to skip the image decoding when the
image is not changed. But that logic is not implemented yet, so I just pass it.

+}
+
+}  // namespace chromeos