Transfer responsibility for providing a close button for a notification to each implementation of M…

ui/arc/notification/arc_custom_notification_item.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 "ui/arc/notification/arc_custom_notification_item.h"
 
 #include <memory>
 #include <string>
 #include <utility>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/arc/notification/arc_custom_notification_view.h"
 #include "ui/message_center/notification.h"
 #include "ui/message_center/notification_types.h"
+#include "ui/message_center/views/custom_notification_content_view_delegate.h"
 
 namespace arc {
 
 namespace {
 
 constexpr char kNotifierId[] = "ARC_NOTIFICATION";
 
 class ArcNotificationDelegate : public message_center::NotificationDelegate {
  public:
   explicit ArcNotificationDelegate(ArcCustomNotificationItem* item)
       : item_(item) {}
 
-  std::unique_ptr<views::View> CreateCustomContent() override {
-    return base::MakeUnique<ArcCustomNotificationView>(item_);
+  std::tuple<

[yoshiki, 2016/12/02 12:15:48]

Instead of using a tuple, how about creating new class which has 2 properties
for views::View and CustomNotificationContentViewDelegate?

Or the following might also be good.
1. Move the code creating ArcCustomNotificationView instance (l.34) to the
constructor.
2. Rename CreateCustomContent() method to GetCustomContentView()
3. Add GetCustomContentViewDelegate() method which returns a delegate instance
to NotificationDelegate class.

IMO, using a tuple here looks somewhat hard to read for me.

[xiyuan, 2016/12/02 17:45:53]

+1 to this, e.g. making struct CustomContent instead of a tuple to hold the two.
NotificationDelegate is expected to create a custom content view that is going
to be owned  by CustomNotificationView later on. It would be hard to split this
into two functions and correctly associate the CustomContentViewDelegat with the
view.

[yhanada, 2016/12/06 02:48:08]

Created |CustomContent| struct for holding the two. Done.

+      std::unique_ptr<views::View>,
+      std::unique_ptr<message_center::CustomNotificationContentViewDelegate>>
+  CreateCustomContent() override {
+    auto view = base::MakeUnique<ArcCustomNotificationView>(item_);
+    auto content_view_delegate = view->CreateContentViewDelegate();
+    return std::make_tuple(std::move(view), std::move(content_view_delegate));
   }
 
  private:
   // The destructor is private since this class is ref-counted.
   ~ArcNotificationDelegate() override {}
 
   ArcCustomNotificationItem* const item_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcNotificationDelegate);
 };
@@ skipping 83 matching lines @@
 }
 
 void ArcCustomNotificationItem::DecrementWindowRefCount() {
   DCHECK_GT(window_ref_count_, 0);
   --window_ref_count_;
   if (window_ref_count_ == 0)
     manager()->CloseNotificationWindow(notification_key());
 }
 
 }  // namespace arc