[Web Payments] Add "pencil" edit button to lists.

chrome/browser/ui/views/payments/payment_request_item_list.cc

 // Copyright 2017 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/ui/views/payments/payment_request_item_list.h"
 
+#include "chrome/app/vector_icons/vector_icons.h"
 #include "chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h"
 #include "chrome/browser/ui/views/payments/payment_request_row_view.h"
 #include "chrome/browser/ui/views/payments/payment_request_views_util.h"
 #include "components/payments/content/payment_request_state.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/button/image_button_factory.h"
 #include "ui/views/controls/image_view.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/grid_layout.h"
 #include "ui/views/vector_icons.h"
 #include "ui/views/view.h"
 
 namespace payments {
 
 namespace {
 
 const SkColor kCheckmarkColor = 0xFF609265;
 
 }  // namespace
 
 PaymentRequestItemList::Item::Item(PaymentRequestSpec* spec,
                                    PaymentRequestState* state,
                                    PaymentRequestItemList* list,
-                                   bool selected)
-    : spec_(spec), state_(state), list_(list), selected_(selected) {}
+                                   bool selected,
+                                   bool show_edit_button)
+    : spec_(spec),
+      state_(state),
+      list_(list),
+      selected_(selected),
+      show_edit_button_(show_edit_button) {}
 
 PaymentRequestItemList::Item::~Item() {}
 
 views::View* PaymentRequestItemList::Item::GetItemView() {
   if (!item_view_) {
     item_view_ = CreateItemView();
     item_view_->set_owned_by_client();
   }
 
   return item_view_.get();
@@ skipping 10 matching lines @@
   if (notify)
     SelectedStateChanged();
 }
 
 std::unique_ptr<views::ImageView> PaymentRequestItemList::Item::CreateCheckmark(
     bool selected) {
   std::unique_ptr<views::ImageView> checkmark =
       base::MakeUnique<views::ImageView>();
   checkmark->set_id(static_cast<int>(DialogViewID::CHECKMARK_VIEW));
   checkmark->set_can_process_events_within_subtree(false);
   checkmark->set_owned_by_client();

[Evan Stade, 2017/05/10 16:06:25]

why is this and the edit button owned by the client?

[anthonyvd, 2017/05/10 16:39:26]

The relationship between views and their controllers in payments code was making
views lifetime non-obvious. Because of this, our convention is to keep
owned_by_client views in unique_ptr members when the controllers keep accessing
the views after their creation. It helps us avoid use-after-free errors and
makes lifetime clearer where it matters.

In this case, the checkmark is made visible/not visible on user interaction and
the edit button is used in a button pressed handler that handles presses from
more than one button.

[Evan Stade, 2017/05/10 17:57:05]

The convention is different for every other piece of UI code in Chrome.
set_owned_by_client should be a rare exception. +sky to weigh in

   checkmark->SetImage(
       gfx::CreateVectorIcon(views::kMenuCheckIcon, kCheckmarkColor));
   checkmark->SetVisible(selected);
   return checkmark;
 }
 
 std::unique_ptr<views::View> PaymentRequestItemList::Item::CreateExtraView() {
   return nullptr;
 }
 
@@ skipping 15 matching lines @@
   views::ColumnSet* columns = layout->AddColumnSet(0);
   columns->AddColumn(views::GridLayout::FILL, views::GridLayout::LEADING, 1,
                      views::GridLayout::USE_PREF, 0, 0);
 
   columns->AddPaddingColumn(1, 0);
 
   // Add a column for the checkmark shown next to the selected profile.
   columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER, 0,
                      views::GridLayout::USE_PREF, 0, 0);
 
-  // Add a column for the extra_view, which comes after the checkmark.
-  columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER, 0,
-                     views::GridLayout::USE_PREF, 0, 0);
+  // The space between the checkmark, extra view, and edit button.
+  constexpr int kExtraViewSpacing = 16;
+  std::unique_ptr<views::View> extra_view = CreateExtraView();
+  if (extra_view) {
+    columns->AddPaddingColumn(0, kExtraViewSpacing);
+    // Add a column for the extra_view, which comes after the checkmark.
+    columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
+                       0, views::GridLayout::USE_PREF, 0, 0);
+  }
+
+  if (show_edit_button_) {
+    edit_button_.reset(views::CreateVectorImageButton(this));
+    edit_button_->set_owned_by_client();
+    views::SetImageFromVectorIcon(edit_button_.get(), kIcEditIcon);
+    constexpr int kEditIconSize = 16;
+    edit_button_->SetSize(gfx::Size(kEditIconSize, kEditIconSize));

[Evan Stade, 2017/05/10 16:06:25]

You should only call View::SetSize during layout.

You can make the column below have a fixed size and pass 16 there.

[anthonyvd, 2017/05/10 16:39:26]

Good point, done.

+    edit_button_->SetFocusBehavior(views::View::FocusBehavior::ALWAYS);
+    edit_button_->set_id(static_cast<int>(DialogViewID::EDIT_ITEM_BUTTON));
+
+    columns->AddPaddingColumn(0, kExtraViewSpacing);
+    // Add a column for the edit_button if it exists.
+    columns->AddColumn(views::GridLayout::TRAILING, views::GridLayout::CENTER,
+                       0, views::GridLayout::USE_PREF, 0, 0);
+  }
 
   layout->StartRow(0, 0);
   content->set_can_process_events_within_subtree(false);
   layout->AddView(content.release());
 
   checkmark_ = CreateCheckmark(selected());
   layout->AddView(checkmark_.get());
 
-  std::unique_ptr<views::View> extra_view = CreateExtraView();
   if (extra_view)
     layout->AddView(extra_view.release());
 
+  if (edit_button_)
+    layout->AddView(edit_button_.get());
+
   return std::move(row);
 }
 
 void PaymentRequestItemList::Item::ButtonPressed(views::Button* sender,
                                                  const ui::Event& event) {
-  if (CanBeSelected()) {
+  if (sender == edit_button_.get()) {
+    EditButtonPressed();
+  } else if (CanBeSelected()) {
     list()->SelectItem(this);
   } else {
     PerformSelectionFallback();
   }
 }
 
 PaymentRequestItemList::PaymentRequestItemList() : selected_item_(nullptr) {}
 
 PaymentRequestItemList::~PaymentRequestItemList() {}
 
@@ skipping 37 matching lines @@
   // It's possible that no item is currently selected, either during list
   // creation or in the middle of the selection operation when the previously
   // selected item has been deselected but the new one isn't selected yet.
   if (selected_item_)
     selected_item_->SetSelected(/*selected=*/false, /*notify=*/true);
 
   selected_item_ = nullptr;
 }
 
 }  // namespace payments