Update the windows UI to support displaying a warning after the

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

 // Copyright (c) 2011 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"
@@ skipping 194 matching lines @@
     // Drop down button is glued to the right of the download shelf.
     drop_down_x_left_ =
         size.width() - normal_drop_down_image_set_.top->width();
     drop_down_x_right_ = size.width();
   }
 
   body_hover_animation_.reset(new ui::SlideAnimation(this));
   drop_hover_animation_.reset(new ui::SlideAnimation(this));
 
   if (download->safety_state() == DownloadItem::DANGEROUS) {
-    tooltip_text_.clear();
-    body_state_ = DANGEROUS;
-    drop_down_state_ = DANGEROUS;
-    save_button_ = new views::NativeTextButton(this,
-        l10n_util::GetStringUTF16(
-            ChromeDownloadManagerDelegate::IsExtensionDownload(download) ?
-                IDS_CONTINUE_EXTENSION_DOWNLOAD : IDS_CONFIRM_DOWNLOAD));
-    save_button_->set_ignore_minimum_size(true);
-    discard_button_ = new views::NativeTextButton(
-        this, l10n_util::GetStringUTF16(IDS_DISCARD_DOWNLOAD));
-    discard_button_->set_ignore_minimum_size(true);
-    AddChildView(save_button_);
-    AddChildView(discard_button_);
-
-    // Ensure the file name is not too long.
-
-    // Extract the file extension (if any).
-    FilePath filename(download->target_name());
-#if defined(OS_POSIX)
-    string16 extension = WideToUTF16(base::SysNativeMBToWide(
-        filename.Extension()));
-#else
-    string16 extension = filename.Extension();
-#endif
-
-    // Remove leading '.'
-    if (extension.length() > 0)
-      extension = extension.substr(1);
-#if defined(OS_POSIX)
-    string16 rootname = WideToUTF16(base::SysNativeMBToWide(
-        filename.RemoveExtension().value()));
-#else
-    string16 rootname = filename.RemoveExtension().value();
-#endif
-
-    // Elide giant extensions (this shouldn't currently be hit, but might
-    // in future, should we ever notice unsafe giant extensions).
-    if (extension.length() > kFileNameMaxLength / 2)
-      ui::ElideString(extension, kFileNameMaxLength / 2, &extension);
-
-    // The dangerous download label text and icon are different
-    // under different cases.
-    string16 dangerous_label;
-    if (download->GetDangerType() == DownloadStateInfo::DANGEROUS_URL) {
-      // Safebrowsing shows the download URL leads to malicious file.
-      warning_icon_ = rb.GetBitmapNamed(IDR_SAFEBROWSING_WARNING);
-      dangerous_label =
-          l10n_util::GetStringUTF16(IDS_PROMPT_UNSAFE_DOWNLOAD_URL);
-    } else {
-      // The download file has dangerous file type (e.g.: an executable).
-      DCHECK(download->GetDangerType() == DownloadStateInfo::DANGEROUS_FILE);
-      warning_icon_ = rb.GetBitmapNamed(IDR_WARNING);
-      if (ChromeDownloadManagerDelegate::IsExtensionDownload(download)) {
-        dangerous_label =
-            l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION);
-      } else {
-        ui::ElideString(rootname,
-                        kFileNameMaxLength - extension.length(),
-                        &rootname);
-        string16 filename = rootname + ASCIIToUTF16(".") + extension;
-        filename = base::i18n::GetDisplayStringInLTRDirectionality(filename);
-        dangerous_label =
-            l10n_util::GetStringFUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD, filename);
-      }
-    }
-
-    dangerous_download_label_ = new views::Label(dangerous_label);
-    dangerous_download_label_->SetMultiLine(true);
-    dangerous_download_label_->SetHorizontalAlignment(
-        views::Label::ALIGN_LEFT);
-    dangerous_download_label_->SetAutoColorReadabilityEnabled(false);
-    AddChildView(dangerous_download_label_);
-    SizeLabelToMinWidth();
+    EnterDangerousMode();
   }

[asanka, 2011/11/18 17:17:18]

Minor nit: No braces needed.

[noelutz, 2011/11/18 17:45:18]

Done.

-
   UpdateAccessibleName();
   set_accessibility_focusable(true);
 
   // Set up our animation.
   StartDownloadProgress();
 }
 
 DownloadItemView::~DownloadItemView() {
   if (context_menu_.get())
     context_menu_->Stop();
@@ skipping 33 matching lines @@
 
 // Update the progress graphic on the icon and our text status label
 // to reflect our current bytes downloaded, time remaining.
 void DownloadItemView::OnDownloadUpdated(DownloadItem* download) {
   DCHECK(download == download_);
 
   if (body_state_ == DANGEROUS &&
       download->safety_state() == DownloadItem::DANGEROUS_BUT_VALIDATED) {
     // We have been approved.
     ClearDangerousMode();
-  }
-
-  string16 status_text = model_->GetStatusText();
-  switch (download_->state()) {
-    case DownloadItem::IN_PROGRESS:
-      download_->is_paused() ? StopDownloadProgress() : StartDownloadProgress();
-      LoadIconIfItemPathChanged();
-      break;
-    case DownloadItem::INTERRUPTED:
-      StopDownloadProgress();
-      complete_animation_.reset(new ui::SlideAnimation(this));
-      complete_animation_->SetSlideDuration(kInterruptedAnimationDurationMs);
-      complete_animation_->SetTweenType(ui::Tween::LINEAR);
-      complete_animation_->Show();
-      SchedulePaint();
-      LoadIcon();
-      break;
-    case DownloadItem::COMPLETE:
-      if (download_->auto_opened()) {
+  } else if (body_state_ != DANGEROUS &&
+      download->safety_state() == DownloadItem::DANGEROUS) {
+    EnterDangerousMode();
+    // Force the shelf to layout again as our size has changed.
+    parent_->Layout();
+    SchedulePaint();
+  } else {
+    string16 status_text = model_->GetStatusText();
+    switch (download_->state()) {
+      case DownloadItem::IN_PROGRESS:
+        download_->is_paused() ?
+            StopDownloadProgress() : StartDownloadProgress();
+        LoadIconIfItemPathChanged();

[asanka, 2011/11/18 17:17:18]

If the download path changes on an IN_PROGRESS download while we showing the
dangerous download prompt and we validate the download, we'll miss out on the
(potentially new) icon until we get another OnDownloadUpdated() notification.

This doesn't currently happen, but I'm not sure we can guarantee that with some
of the upcoming extensions API work.

[noelutz, 2011/11/18 17:45:18]

Are you suggesting I should change something here?  Also, I'm not entirely sure
I follow your point.  If the download path changes on an IN_PROGRESS download
while we show the warning I believe this code is still called and
LoadIconIfItemPathChanged() will be called.  Isn't that what we want?

[Randy Smith (Not in Mondays), 2011/11/18 18:15:26]

Could this be fixed by putting a LoadIconIfItemPathChanged in the Dangerous
conditional, above?  Also, isn't this vulnerability there before Noe's change? 
(In the sense that you have to loosen the currently fairly tight linkage between
path changes and transitions into/out of the dangerous state for it to happen,
and if you do that, I think we have the same problem in the current code).

More generally, could you spell out a use case you think would trigger this
problem, just so I'm sure I have a good understanding of it?

[asanka, 2011/11/18 18:19:01]

The code in the 'else' block here used to run immediately after
ClearDangerousMode().  It doesn't anymore.  I was thinking about potential
behavior changes due to that.

We don't automatically call OnDownloadUpdated() immediately after the path
changes, but there's a state change soon afterwards which winds up sending an
update.  So this code is safe.

Apologies for the confusion.

+        break;
+      case DownloadItem::INTERRUPTED:
+        StopDownloadProgress();
+        complete_animation_.reset(new ui::SlideAnimation(this));
+        complete_animation_->SetSlideDuration(kInterruptedAnimationDurationMs);
+        complete_animation_->SetTweenType(ui::Tween::LINEAR);
+        complete_animation_->Show();
+        SchedulePaint();
+        LoadIcon();
+        break;
+      case DownloadItem::COMPLETE:
+        if (download_->auto_opened()) {
+          parent_->RemoveDownloadView(this);  // This will delete us!
+          return;
+        }
+        StopDownloadProgress();
+        complete_animation_.reset(new ui::SlideAnimation(this));
+        complete_animation_->SetSlideDuration(kCompleteAnimationDurationMs);
+        complete_animation_->SetTweenType(ui::Tween::LINEAR);
+        complete_animation_->Show();
+        SchedulePaint();
+        LoadIcon();
+        break;
+      case DownloadItem::CANCELLED:
+        StopDownloadProgress();
+        LoadIcon();
+        break;
+      case DownloadItem::REMOVING:
         parent_->RemoveDownloadView(this);  // This will delete us!
         return;
-      }
-      StopDownloadProgress();
-      complete_animation_.reset(new ui::SlideAnimation(this));
-      complete_animation_->SetSlideDuration(kCompleteAnimationDurationMs);
-      complete_animation_->SetTweenType(ui::Tween::LINEAR);
-      complete_animation_->Show();
-      SchedulePaint();
-      LoadIcon();
-      break;
-    case DownloadItem::CANCELLED:
-      StopDownloadProgress();
-      LoadIcon();
-      break;
-    case DownloadItem::REMOVING:
-      parent_->RemoveDownloadView(this);  // This will delete us!
-      return;
-    default:
-      NOTREACHED();
+      default:
+        NOTREACHED();
+    }
+    status_text_ = status_text;
   }
-
-  status_text_ = status_text;
   UpdateAccessibleName();
 
   // We use the parent's (DownloadShelfView's) SchedulePaint, since there
   // are spaces between each DownloadItemView that the parent is responsible
   // for painting.
   parent()->SchedulePaint();
 }
 
 void DownloadItemView::OnDownloadOpened(DownloadItem* download) {
   disabled_while_opening_ = true;
@@ skipping 581 matching lines @@
   // Remove the views used by the dangerous mode.
   RemoveChildView(save_button_);
   delete save_button_;
   save_button_ = NULL;
   RemoveChildView(discard_button_);
   delete discard_button_;
   discard_button_ = NULL;
   RemoveChildView(dangerous_download_label_);
   delete dangerous_download_label_;
   dangerous_download_label_ = NULL;
+  dangerous_download_label_sized_ = false;
 
   // Set the accessible name back to the status and filename instead of the
   // download warning.
   UpdateAccessibleName();
 
   // We need to load the icon now that the download_ has the real path.
   LoadIcon();
   tooltip_text_ = download_->GetFileNameToReportUser().LossyDisplayName();
 
   // Force the shelf to layout again as our size has changed.
   parent_->Layout();
   parent_->SchedulePaint();
 }
 
+void DownloadItemView::EnterDangerousMode() {
+  DCHECK(body_state_ != DANGEROUS && drop_down_state_ != DANGEROUS);
+  tooltip_text_.clear();
+  body_state_ = DANGEROUS;
+  drop_down_state_ = DANGEROUS;
+  save_button_ = new views::NativeTextButton(this,
+      l10n_util::GetStringUTF16(
+           ChromeDownloadManagerDelegate::IsExtensionDownload(download_) ?
+              IDS_CONTINUE_EXTENSION_DOWNLOAD : IDS_CONFIRM_DOWNLOAD));
+  save_button_->set_ignore_minimum_size(true);
+  discard_button_ = new views::NativeTextButton(
+      this, l10n_util::GetStringUTF16(IDS_DISCARD_DOWNLOAD));
+  discard_button_->set_ignore_minimum_size(true);
+  AddChildView(save_button_);
+  AddChildView(discard_button_);
+
+  // Ensure the file name is not too long.
+
+  // Extract the file extension (if any).
+  FilePath filename(download_->target_name());
+#if defined(OS_POSIX)
+  string16 extension = WideToUTF16(base::SysNativeMBToWide(
+      filename.Extension()));
+#else
+  string16 extension = filename.Extension();
+#endif
+
+  // Remove leading '.'
+  if (extension.length() > 0)
+    extension = extension.substr(1);
+#if defined(OS_POSIX)
+  string16 rootname = WideToUTF16(base::SysNativeMBToWide(
+      filename.RemoveExtension().value()));
+#else
+  string16 rootname = filename.RemoveExtension().value();
+#endif
+
+  // Elide giant extensions (this shouldn't currently be hit, but might
+  // in future, should we ever notice unsafe giant extensions).
+  if (extension.length() > kFileNameMaxLength / 2)
+    ui::ElideString(extension, kFileNameMaxLength / 2, &extension);
+
+  ResourceBundle &rb = ResourceBundle::GetSharedInstance();
+  // The dangerous download label text and icon are different
+  // under different cases.
+  string16 dangerous_label;
+  if (download_->GetDangerType() == DownloadStateInfo::DANGEROUS_URL ||
+    download_->GetDangerType() == DownloadStateInfo::DANGEROUS_CONTENT) {
+    // TODO(noelutz): add the target filename to the warning message in the
+    // case of a dangerous content warning.
+    // Safebrowsing shows the download URL or content leads to malicious file.
+    warning_icon_ = rb.GetBitmapNamed(IDR_SAFEBROWSING_WARNING);
+    dangerous_label =
+        l10n_util::GetStringUTF16(IDS_PROMPT_UNSAFE_DOWNLOAD_URL);
+  } else {
+    // The download file has dangerous file type (e.g.: an executable).
+    DCHECK(download_->GetDangerType() == DownloadStateInfo::DANGEROUS_FILE);
+    warning_icon_ = rb.GetBitmapNamed(IDR_WARNING);
+    if (ChromeDownloadManagerDelegate::IsExtensionDownload(download_)) {
+      dangerous_label =
+          l10n_util::GetStringUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD_EXTENSION);
+    } else {
+      ui::ElideString(rootname,
+                      kFileNameMaxLength - extension.length(),
+                      &rootname);
+      string16 filename = rootname + ASCIIToUTF16(".") + extension;
+      filename = base::i18n::GetDisplayStringInLTRDirectionality(filename);
+      dangerous_label =
+          l10n_util::GetStringFUTF16(IDS_PROMPT_DANGEROUS_DOWNLOAD, filename);
+    }
+  }
+
+  dangerous_download_label_ = new views::Label(dangerous_label);
+  dangerous_download_label_->SetMultiLine(true);
+  dangerous_download_label_->SetHorizontalAlignment(
+      views::Label::ALIGN_LEFT);
+  dangerous_download_label_->SetAutoColorReadabilityEnabled(false);
+  AddChildView(dangerous_download_label_);
+  SizeLabelToMinWidth();
+}
+
 gfx::Size DownloadItemView::GetButtonSize() {
   DCHECK(save_button_ && discard_button_);
   gfx::Size size;
 
   // We cache the size when successfully retrieved, not for performance reasons
   // but because if this DownloadItemView is being animated while the tab is
   // not showing, the native buttons are not parented and their preferred size
   // is 0, messing-up the layout.
   if (cached_button_size_.width() != 0)
     return cached_button_size_;
@@ skipping 99 matching lines @@
   // If the name has changed, notify assistive technology that the name
   // has changed so they can announce it immediately.
   if (new_name != accessible_name_) {
     accessible_name_ = new_name;
     if (GetWidget()) {
       GetWidget()->NotifyAccessibilityEvent(
           this, ui::AccessibilityTypes::EVENT_NAME_CHANGED, true);
     }
   }
 }