[M102][DownloadBubble] Use visibility instead of construction/destruc…

…tion

When showing the main or the security subpage, use visibility instead
of construction/destruction. Added methods to update subpage.

(cherry picked from commit 9f58d3a)

Bug: 1316740
Bug: 1315395
Change-Id: I970016445e169b472ed02e7cd294099bc4a3e64e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3597896
Reviewed-by: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Rohit Bhatia <bhatiarohit@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#995434}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3605901
Auto-Submit: Rohit Bhatia <bhatiarohit@google.com>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#177}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}

chrome/browser/ui/views/download/bubble/download_bubble_row_view.cc

@@ -316,10 +316,7 @@ views::View* DownloadBubbleRowView::GetTooltipHandlerForPoint(
 
 void DownloadBubbleRowView::OnMainButtonPressed() {
   if (ui_info_.has_subpage) {
-    SetEnabled(false);
-    model_->RemoveObserver(this);
-    navigation_handler_->OpenSecurityDialog(std::move(model_), ui_info_);
-    // |this| is deleted now.
+    navigation_handler_->OpenSecurityDialog(this);
   } else {
     DownloadCommands(model_->GetWeakPtr())
         .ExecuteCommand(DownloadCommands::OPEN_WHEN_COMPLETE);

chrome/browser/ui/views/download/bubble/download_bubble_row_view.h

@@ -61,6 +61,10 @@ class DownloadBubbleRowView : public HoverButton,
   // views::ViewTargeterDelegate:
   views::View* TargetForRect(views::View* root, const gfx::Rect& rect) override;
 
+  DownloadUIModel* model() { return model_.get(); }
+
+  DownloadUIModel::BubbleUIInfo& ui_info() { return ui_info_; }
+
  protected:
   // Overrides ui::LayerDelegate:
   void OnDeviceScaleFactorChanged(float old_device_scale_factor,

chrome/browser/ui/views/download/bubble/download_bubble_security_view.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/download/bubble/download_bubble_controller.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/views/chrome_layout_provider.h"
+#include "chrome/browser/ui/views/download/bubble/download_bubble_row_view.h"
 #include "chrome/browser/ui/views/download/bubble/download_toolbar_button_view.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
@@ -25,7 +26,15 @@
 
 namespace {
 constexpr int kCheckboxHeight = 32;
-constexpr int kBuffer = 40;
+constexpr auto kCommandToButtons = base::MakeFixedFlatMap<
+    DownloadCommands::Command,
+    raw_ptr<views::MdTextButton> DownloadBubbleSecurityView::*>(
+    {{DownloadCommands::DISCARD, &DownloadBubbleSecurityView::discard_button_},
+     {DownloadCommands::KEEP, &DownloadBubbleSecurityView::keep_button_},
+     {DownloadCommands::DEEP_SCAN,
+      &DownloadBubbleSecurityView::deep_scan_button_},
+     {DownloadCommands::BYPASS_DEEP_SCANNING,
+      &DownloadBubbleSecurityView::bypass_deep_scan_button_}});
 }  // namespace
 
 void DownloadBubbleSecurityView::AddHeader() {
@@ -48,20 +57,19 @@ void DownloadBubbleSecurityView::AddHeader() {
   back_button->SetProperty(views::kCrossAxisAlignmentKey,
                            views::LayoutAlignment::kStart);
 
-  auto* title = header->AddChildView(std::make_unique<views::Label>(
-      model_->GetFileNameToReportUser().LossyDisplayName(),
-      views::style::CONTEXT_DIALOG_TITLE, views::style::STYLE_PRIMARY));
-  title->SetProperty(
+  title_ = header->AddChildView(std::make_unique<views::Label>(
+      std::u16string(), views::style::CONTEXT_DIALOG_TITLE,
+      views::style::STYLE_PRIMARY));
+  title_->SetProperty(
       views::kFlexBehaviorKey,
       views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
                                views::MaximumFlexSizeRule::kUnbounded,
-                               /*adjust_height_for_width=*/true)
-          .WithWeight(1));
+                               /*adjust_height_for_width=*/false));
   const int icon_label_spacing = ChromeLayoutProvider::Get()->GetDistanceMetric(
       views::DISTANCE_RELATED_LABEL_HORIZONTAL);
-  title->SetProperty(views::kMarginsKey,
-                     gfx::Insets::VH(0, icon_label_spacing));
-  title->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  title_->SetProperty(views::kMarginsKey,
+                      gfx::Insets::VH(0, icon_label_spacing));
+  title_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
 
   auto* close_button =
       header->AddChildView(views::CreateVectorImageButtonWithNativeTheme(
@@ -75,102 +83,150 @@ void DownloadBubbleSecurityView::AddHeader() {
                             views::LayoutAlignment::kStart);
 }
 
+void DownloadBubbleSecurityView::UpdateHeader() {
+  title_->SetText(download_row_view_->model()
+                      ->GetFileNameToReportUser()
+                      .LossyDisplayName());
+}
+
 void DownloadBubbleSecurityView::CloseBubble() {
   navigation_handler_->CloseDialog(
       views::Widget::ClosedReason::kCloseButtonClicked);
 }
 
 void DownloadBubbleSecurityView::OnCheckboxClicked() {
   first_button_->SetEnabled(checkbox_->GetChecked());
-  first_button_->SetEnabledTextColors(
-      GetColorProvider()->GetColor(info_.secondary_color));
+  first_button_->SetEnabledTextColors(GetColorProvider()->GetColor(
+      download_row_view_->ui_info().secondary_color));
 }
 
-void DownloadBubbleSecurityView::AddIconAndText() {
+void DownloadBubbleSecurityView::UpdateIconAndText() {
+  DownloadUIModel::BubbleUIInfo& ui_info = download_row_view_->ui_info();
+  icon_->SetImage(ui::ImageModel::FromVectorIcon(
+      *(ui_info.icon_model_override), ui_info.secondary_color,
+      GetLayoutConstant(DOWNLOAD_ICON_SIZE)));
+
+  styled_label_->SetText(ui_info.warning_summary);
+  // The label defaults to a single line, which would force the dialog wider;
+  // instead give it a width that's the minimum we want it to have. Then the
+  // Layout will stretch it back out into any additional space available.
+  // The side margin is added twice, once in the bubble, and then for each
+  // row view.
   const int side_margin = ChromeLayoutProvider::Get()->GetDistanceMetric(
       views::DISTANCE_RELATED_CONTROL_VERTICAL);
   const int icon_label_spacing = ChromeLayoutProvider::Get()->GetDistanceMetric(
       views::DISTANCE_RELATED_LABEL_HORIZONTAL);
   const int bubble_width = ChromeLayoutProvider::Get()->GetDistanceMetric(
       views::DISTANCE_BUBBLE_PREFERRED_WIDTH);
+  const int min_label_width =
+      bubble_width - side_margin * 4 - GetLayoutConstant(DOWNLOAD_ICON_SIZE) -
+      GetLayoutInsets(DOWNLOAD_ICON).width() - icon_label_spacing;
+  styled_label_->SizeToFit(min_label_width);
+
+  checkbox_->SetVisible(ui_info.has_checkbox);
+  if (ui_info.has_checkbox) {
+    checkbox_->SetText(ui_info.checkbox_label);
+  }
+}
+
+void DownloadBubbleSecurityView::AddIconAndText() {
+  const int side_margin = ChromeLayoutProvider::Get()->GetDistanceMetric(
+      views::DISTANCE_RELATED_CONTROL_VERTICAL);
+  const int icon_label_spacing = ChromeLayoutProvider::Get()->GetDistanceMetric(
+      views::DISTANCE_RELATED_LABEL_HORIZONTAL);
 
   auto* icon_text_row = AddChildView(std::make_unique<views::View>());
   icon_text_row->SetLayoutManager(std::make_unique<views::FlexLayout>())
       ->SetOrientation(views::LayoutOrientation::kHorizontal)
       .SetCrossAxisAlignment(views::LayoutAlignment::kStart);
   icon_text_row->SetProperty(views::kMarginsKey, gfx::Insets(side_margin));
 
-  auto* icon =
-      icon_text_row->AddChildView(std::make_unique<views::ImageView>());
-  icon->SetProperty(views::kMarginsKey, GetLayoutInsets(DOWNLOAD_ICON));
-  icon->SetImage(ui::ImageModel::FromVectorIcon(
-      *info_.icon_model_override, info_.secondary_color,
-      GetLayoutConstant(DOWNLOAD_ICON_SIZE)));
+  icon_ = icon_text_row->AddChildView(std::make_unique<views::ImageView>());
+  icon_->SetProperty(views::kMarginsKey, GetLayoutInsets(DOWNLOAD_ICON));
 
   auto* wrapper = icon_text_row->AddChildView(std::make_unique<views::View>());
   wrapper->SetLayoutManager(std::make_unique<views::FlexLayout>())
       ->SetOrientation(views::LayoutOrientation::kVertical)
       .SetCrossAxisAlignment(views::LayoutAlignment::kStart);
   wrapper->SetProperty(views::kMarginsKey,
-                       gfx::Insets::VH(0, icon_label_spacing));
+                       gfx::Insets::TLBR(0, icon_label_spacing, 0, 0));
   wrapper->SetProperty(views::kCrossAxisAlignmentKey,
                        views::LayoutAlignment::kStretch);
   wrapper->SetProperty(
       views::kFlexBehaviorKey,
-      views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToZero,
+      views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToMinimum,
                                views::MaximumFlexSizeRule::kUnbounded,
-                               /*adjust_height_for_width=*/true)
-          .WithWeight(1));
-
-  auto* styled_label =
-      wrapper->AddChildView(std::make_unique<views::StyledLabel>());
-  styled_label->SetProperty(views::kCrossAxisAlignmentKey,
-                            views::LayoutAlignment::kStretch);
-  styled_label->SetTextContext(views::style::CONTEXT_DIALOG_BODY_TEXT);
-  styled_label->SetText(info_.warning_summary);
-  styled_label->SetProperty(
+                               /*adjust_height_for_width=*/true));
+
+  styled_label_ = wrapper->AddChildView(std::make_unique<views::StyledLabel>());
+  styled_label_->SetProperty(views::kCrossAxisAlignmentKey,
+                             views::LayoutAlignment::kStretch);
+  styled_label_->SetTextContext(views::style::CONTEXT_DIALOG_BODY_TEXT);
+  styled_label_->SetProperty(
       views::kFlexBehaviorKey,
       views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToMinimum,
-                               views::MaximumFlexSizeRule::kPreferred,
-                               /*adjust_height_for_width=*/true)
-          .WithWeight(1));
-  // The label defaults to a single line, which would force the dialog wider;
-  // instead give it a width that's the minimum we want it to have. Then the
-  // Layout will stretch it back out into any additional space available.
-  // Reduce by extra buffer so it has space for word wrapping.
-  const int min_label_width =
-      bubble_width - side_margin * 4 - icon->GetImageModel().Size().width() -
-      2 * GetLayoutInsets(DOWNLOAD_ICON).width() - icon_label_spacing - kBuffer;
-  styled_label->SizeToFit(min_label_width);
-
-  if (info_.has_checkbox) {
-    checkbox_ = wrapper->AddChildView(std::make_unique<views::Checkbox>(
-        info_.checkbox_label,
-        base::BindRepeating(&DownloadBubbleSecurityView::OnCheckboxClicked,
-                            base::Unretained(this))));
-    checkbox_->SetMultiLine(true);
-    checkbox_->SetProperty(
-        views::kMarginsKey,
-        gfx::Insets::VH(ChromeLayoutProvider::Get()->GetDistanceMetric(
-                            views::DISTANCE_RELATED_CONTROL_VERTICAL),
-                        0));
-    checkbox_->SetProperty(views::kCrossAxisAlignmentKey,
-                           views::LayoutAlignment::kStretch);
-    checkbox_->SetProperty(
-        views::kFlexBehaviorKey,
-        views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred,
-                                 views::MaximumFlexSizeRule::kUnbounded,
-                                 /*adjust_height_for_width=*/false)
-            .WithWeight(1));
-    // Set min height for checkbox, so that it can layout label accordingly.
-    checkbox_->SetMinSize(gfx::Size(0, kCheckboxHeight));
-  }
+                               views::MaximumFlexSizeRule::kUnbounded,
+                               /*adjust_height_for_width=*/true));
+
+  checkbox_ = wrapper->AddChildView(std::make_unique<views::Checkbox>(
+      std::u16string(),
+      base::BindRepeating(&DownloadBubbleSecurityView::OnCheckboxClicked,
+                          base::Unretained(this))));
+  checkbox_->SetMultiLine(true);
+  checkbox_->SetProperty(
+      views::kMarginsKey,
+      gfx::Insets::VH(ChromeLayoutProvider::Get()->GetDistanceMetric(
+                          views::DISTANCE_RELATED_CONTROL_VERTICAL),
+                      0));
+  checkbox_->SetProperty(views::kCrossAxisAlignmentKey,
+                         views::LayoutAlignment::kStretch);
+  checkbox_->SetProperty(
+      views::kFlexBehaviorKey,
+      views::FlexSpecification(views::MinimumFlexSizeRule::kScaleToMinimum,
+                               views::MaximumFlexSizeRule::kUnbounded,
+                               /*adjust_height_for_width=*/true));
+  // Set min height for checkbox, so that it can layout label accordingly.
+  checkbox_->SetMinSize(gfx::Size(0, kCheckboxHeight));
 }
 
 void DownloadBubbleSecurityView::ProcessButtonClick(
     DownloadCommands::Command command) {
-  bubble_controller_->ProcessDownloadButtonPress(model_.get(), command);
+  // First open primary dialog, and then execute the command. If a deletion
+  // happens leading to closure of the bubble, it will be called after primary
+  // dialog is opened.
   navigation_handler_->OpenPrimaryDialog();
+  bubble_controller_->ProcessDownloadButtonPress(download_row_view_->model(),
+                                                 command);
+}
+
+views::MdTextButton* DownloadBubbleSecurityView::GetButtonForCommand(
+    DownloadCommands::Command command) {
+  auto* button_iter = kCommandToButtons.find(command);
+  return (button_iter != kCommandToButtons.end()) ? this->*(button_iter->second)
+                                                  : nullptr;
+}
+
+void DownloadBubbleSecurityView::UpdateButtons() {
+  discard_button_->SetVisible(false);
+  keep_button_->SetVisible(false);
+  deep_scan_button_->SetVisible(false);
+  bypass_deep_scan_button_->SetVisible(false);
+  DownloadUIModel::BubbleUIInfo& ui_info = download_row_view_->ui_info();
+  if (ui_info.has_first_button) {
+    first_button_ = GetButtonForCommand(ui_info.first_button_command);
+    first_button_->SetText(ui_info.first_button_label);
+    if (ui_info.has_checkbox) {
+      first_button_->SetEnabled(false);
+    }
+    first_button_->SetVisible(true);
+  }
+  if (ui_info.has_second_button) {
+    views::MdTextButton* second_button =
+        GetButtonForCommand(ui_info.second_button_command);
+    second_button->SetText(ui_info.second_button_label);
+    second_button->SetVisible(true);
+    second_button->SetProminent(true);
+  }
 }
 
 void DownloadBubbleSecurityView::AddButtons() {
@@ -183,44 +239,44 @@ void DownloadBubbleSecurityView::AddButtons() {
       gfx::Insets(ChromeLayoutProvider::Get()->GetDistanceMetric(
           views::DISTANCE_RELATED_CONTROL_VERTICAL)));
 
-  if (info_.has_first_button) {
-    first_button_ =
-        button_row->AddChildView(std::make_unique<views::MdTextButton>(
-            base::BindRepeating(&DownloadBubbleSecurityView::ProcessButtonClick,
-                                base::Unretained(this),
-                                info_.first_button_command),
-            info_.first_button_label));
-    first_button_->SetProperty(
-        views::kMarginsKey,
-        gfx::Insets::VH(0, ChromeLayoutProvider::Get()->GetDistanceMetric(
-                               views::DISTANCE_RELATED_CONTROL_HORIZONTAL)));
-
-    if (info_.has_checkbox) {
-      first_button_->SetEnabled(false);
-    }
-  }
+  gfx::Insets button_margin =
+      gfx::Insets::VH(0, ChromeLayoutProvider::Get()->GetDistanceMetric(
+                             views::DISTANCE_RELATED_CONTROL_HORIZONTAL));
 
-  if (info_.has_second_button) {
-    raw_ptr<views::MdTextButton> second_button =
+  auto add_button_for_command = [button_row, button_margin,
+                                 this](DownloadCommands::Command command) {
+    auto* button =
         button_row->AddChildView(std::make_unique<views::MdTextButton>(
             base::BindRepeating(&DownloadBubbleSecurityView::ProcessButtonClick,
-                                base::Unretained(this),
-                                info_.second_button_command),
-            info_.second_button_label));
-    second_button->SetProminent(true);
-  }
+                                base::Unretained(this), command),
+            std::u16string()));
+    button->SetProperty(views::kMarginsKey, button_margin);
+    return button;
+  };
+
+  // The buttons come in this order KEEP, DISCARD, BYPASS_DEEP_SCANNING,
+  // DEEP_SCAN. Reorder buttons in runtime if required.
+  keep_button_ = add_button_for_command(DownloadCommands::KEEP);
+  discard_button_ = add_button_for_command(DownloadCommands::DISCARD);
+  bypass_deep_scan_button_ =
+      add_button_for_command(DownloadCommands::BYPASS_DEEP_SCANNING);
+  deep_scan_button_ = add_button_for_command(DownloadCommands::DEEP_SCAN);
+}
+
+void DownloadBubbleSecurityView::UpdateSecurityView(
+    DownloadBubbleRowView* download_row_view) {
+  download_row_view_ = download_row_view;
+  DCHECK(download_row_view_->model());
+  UpdateHeader();
+  UpdateIconAndText();
+  UpdateButtons();
 }
 
 DownloadBubbleSecurityView::DownloadBubbleSecurityView(
-    DownloadUIModel::DownloadUIModelPtr model,
-    DownloadUIModel::BubbleUIInfo info,
     DownloadBubbleUIController* bubble_controller,
     DownloadBubbleNavigationHandler* navigation_handler)
-    : model_(std::move(model)),
-      info_(info),
-      bubble_controller_(bubble_controller),
+    : bubble_controller_(bubble_controller),
       navigation_handler_(navigation_handler) {
-  DCHECK(model_.get());
   SetLayoutManager(std::make_unique<views::FlexLayout>())
       ->SetOrientation(views::LayoutOrientation::kVertical);
   AddHeader();