Implement new dangerous download reporting dialog for UNCOMMON_DOWNLOAD, in Views

chrome/browser/ui/views/download/download_item_view.cc

 // Copyright (c) 2012 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/download/download_item_view.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/i18n/break_iterator.h"
 #include "base/i18n/rtl.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_stats.h"
 #include "chrome/browser/download/drag_download_item.h"
+#include "chrome/browser/prefs/pref_service_syncable.h"
 #include "chrome/browser/safe_browsing/download_feedback_service.h"
 #include "chrome/browser/safe_browsing/download_protection_service.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/themes/theme_properties.h"
+#include "chrome/browser/ui/views/download/download_feedback_dialog_view.h"
 #include "chrome/browser/ui/views/download/download_shelf_context_menu_view.h"
 #include "chrome/browser/ui/views/download/download_shelf_view.h"
 #include "content/public/browser/download_danger_type.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "third_party/icu/source/common/unicode/uchar.h"
 #include "ui/base/accessibility/accessible_view_state.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/theme_provider.h"
@@ skipping 504 matching lines @@
     return;
   }
 
   // WARNING: all end states after this point delete |this|.
   DCHECK_EQ(discard_button_, sender);
   if (model_.IsMalicious()) {
     UMA_HISTOGRAM_LONG_TIMES("clickjacking.dismiss_download", warning_duration);
     shelf_->RemoveDownloadView(this);
     return;
   }
-  if (model_.ShouldAllowDownloadFeedback() && BeginDownloadFeedback())
+  UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download", warning_duration);
+  if (model_.ShouldAllowDownloadFeedback()) {
+    DownloadFeedbackDialogView::DownloadReportingStatus pref_value =
+        static_cast<DownloadFeedbackDialogView::DownloadReportingStatus>(
+            shelf_->browser()->profile()->GetPrefs()->GetInteger(
+                prefs::kSafeBrowsingDownloadReportingEnabled));
+    switch (pref_value) {
+      case DownloadFeedbackDialogView::kDialogNotYetShown:
+        DownloadFeedbackDialogView::Show(
+            shelf_->get_parent()->GetNativeWindow(),
+            shelf_->browser()->profile(),
+            base::Bind(
+                &DownloadItemView::PossiblySubmitDownloadToFeedbackService,
+                weak_ptr_factory_.GetWeakPtr()));
+        break;
+      case DownloadFeedbackDialogView::kDownloadReportingEnabled:

[Peter Kasting, 2014/02/05 19:47:08]

Nit: I might put a newline above this line, as well as above the kMaxValue case
line below, just to make the logical blocks more visually distinct.

[felt, 2014/02/05 19:57:26]

Done.

+      case DownloadFeedbackDialogView::kDownloadReportingDisabled:
+        PossiblySubmitDownloadToFeedbackService(pref_value);
+        break;
+      case DownloadFeedbackDialogView::kMaxValue:
+        NOTREACHED();
+    }
     return;
-  UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download", warning_duration);
+  }
   download()->Remove();
 }
 
 void DownloadItemView::AnimationProgressed(const gfx::Animation* animation) {
   // We don't care if what animation (body button/drop button/complete),
   // is calling back, as they all have to go through the same paint call.
   SchedulePaint();
 }
 
 void DownloadItemView::OnPaint(gfx::Canvas* canvas) {
@@ skipping 317 matching lines @@
 void DownloadItemView::OpenDownload() {
   DCHECK(!IsShowingWarningDialog());
   // We're interested in how long it takes users to open downloads.  If they
   // open downloads super quickly, we should be concerned about clickjacking.
   UMA_HISTOGRAM_LONG_TIMES("clickjacking.open_download",
                            base::Time::Now() - creation_time_);
   download()->OpenDownload();
   UpdateAccessibleName();
 }
 
-bool DownloadItemView::BeginDownloadFeedback() {
+bool DownloadItemView::SubmitDownloadToFeedbackService() {
 #if defined(FULL_SAFE_BROWSING)
   SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
   if (!sb_service)
     return false;
   safe_browsing::DownloadProtectionService* download_protection_service =
       sb_service->download_protection_service();
   if (!download_protection_service)
     return false;
-  base::TimeDelta warning_duration = base::TimeDelta();
-  if (!time_download_warning_shown_.is_null())
-    warning_duration = base::Time::Now() - time_download_warning_shown_;
-  UMA_HISTOGRAM_LONG_TIMES("clickjacking.report_and_discard_download",
-                           warning_duration);
   download_protection_service->feedback_service()->BeginFeedbackForDownload(
       download());
   // WARNING: we are deleted at this point.  Don't access 'this'.
   return true;
 #else
   NOTREACHED();
   return false;
 #endif
 }
 
+void DownloadItemView::PossiblySubmitDownloadToFeedbackService(
+    DownloadFeedbackDialogView::DownloadReportingStatus status) {
+  if (status != DownloadFeedbackDialogView::kDownloadReportingEnabled ||
+      !SubmitDownloadToFeedbackService()) {

[Peter Kasting, 2014/02/05 19:47:08]

Nit: {} optional

[felt, 2014/02/05 19:57:26]

I like brackets when the condition is split across lines, so keeping {}.

+    download()->Remove();
+  }
+  // WARNING: 'this' is deleted at this point. Don't access 'this'.
+}
+
 void DownloadItemView::LoadIcon() {
   IconManager* im = g_browser_process->icon_manager();
   last_download_item_path_ = download()->GetTargetFilePath();
   im->LoadIcon(last_download_item_path_,
                IconLoader::SMALL,
                base::Bind(&DownloadItemView::OnExtractIconComplete,
                           base::Unretained(this)),
                &cancelable_task_tracker_);
 }
 
@@ skipping 207 matching lines @@
   body_state_ = NORMAL;
   drop_down_state_ = NORMAL;
   if (mode_ == DANGEROUS_MODE) {
     save_button_ = new views::LabelButton(
         this, model_.GetWarningConfirmButtonText());
     save_button_->SetStyle(views::Button::STYLE_BUTTON);
     AddChildView(save_button_);
   }
   int discard_button_message = model_.IsMalicious() ?
       IDS_DISMISS_DOWNLOAD : IDS_DISCARD_DOWNLOAD;
-  if (!model_.IsMalicious() && model_.ShouldAllowDownloadFeedback())
-    discard_button_message = IDS_REPORT_AND_DISCARD_DOWNLOAD;
   discard_button_ = new views::LabelButton(
       this, l10n_util::GetStringUTF16(discard_button_message));
   discard_button_->SetStyle(views::Button::STYLE_BUTTON);
   AddChildView(discard_button_);
 
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   switch (danger_type) {
     case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_URL:
     case content::DOWNLOAD_DANGER_TYPE_DANGEROUS_CONTENT:
     case content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT:
@@ skipping 163 matching lines @@
 void DownloadItemView::AnimateStateTransition(State from, State to,
                                               gfx::SlideAnimation* animation) {
   if (from == NORMAL && to == HOT) {
     animation->Show();
   } else if (from == HOT && to == NORMAL) {
     animation->Hide();
   } else if (from != to) {
     animation->Reset((to == HOT) ? 1.0 : 0.0);
   }
 }