Show download interrupt reason in a download item tooltip.

When the user hovers over a download item on the shelf that correspond to an interrupted download, the reason for the interruption is displayed in a tooltip.

(Original CL by ahendrickson http://codereview.chromium.org/9968090/ )

BUG=121698
TEST=None


Review URL: http://codereview.chromium.org/10169025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133907 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/download/download_item_model.cc

@@ -150,6 +150,20 @@ string16 DownloadItemModel::GetStatusText() {
   return status_text;
 }
 
+string16 DownloadItemModel::GetTooltipText(const gfx::Font& font,
+                                           int max_width) const {
+  string16 tooltip = ui::ElideFilename(
+      download_->GetFileNameToReportUser(), font, max_width);
+  content::DownloadInterruptReason reason = download_->GetLastReason();
+  if (download_->GetState() == DownloadItem::INTERRUPTED &&
+      reason != content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED) {
+    tooltip += ASCIIToUTF16("\n");
+    tooltip += ui::ElideText(InterruptReasonStatusMessage(reason),
+                             font, max_width, ui::ELIDE_AT_END);
+  }
+  return tooltip;
+}
+
 int DownloadItemModel::PercentComplete() const {
 #if defined(OS_CHROMEOS)
   // For GData uploads, progress is based on the number of bytes

chrome/browser/download/download_item_model.h

@@ -32,9 +32,21 @@ class BaseDownloadItemModel {
   // Cancel the task corresponding to the item.
   virtual void CancelTask() = 0;
 
-  // Get the status text to display.
+  // Returns a short one-line status string for the download.
   virtual string16 GetStatusText() = 0;
 
+  // Returns a string suitable for use as a tooltip. For a regular download, the
+  // tooltip is the filename. For an interrupted download, the string states the
+  // filename and a short description of the reason for interruption. For
+  // example:
+  //    Report.pdf
+  //    Network disconnected
+  // |font| and |max_width| are used to elide the filename and/or interrupt
+  // reason as necessary to keep the width of the tooltip text under
+  // |max_width|. The tooltip will be at most 2 lines.
+  virtual string16 GetTooltipText(const gfx::Font& font,
+                                  int max_width) const = 0;
+
   // Rough percent complete. Returns -1 if the progress is unknown.
   virtual int PercentComplete() const = 0;
 
@@ -75,6 +87,8 @@ class DownloadItemModel : public BaseDownloadItemModel {
   // BaseDownloadItemModel
   virtual void CancelTask() OVERRIDE;
   virtual string16 GetStatusText() OVERRIDE;
+  virtual string16 GetTooltipText(const gfx::Font& font,
+                                  int max_width) const OVERRIDE;
   virtual int PercentComplete() const OVERRIDE;
   virtual string16 GetWarningText(const gfx::Font& font,
                                   int base_width) OVERRIDE;

chrome/browser/download/download_item_model_unittest.cc

@@ -4,17 +4,21 @@
 
 #include "chrome/browser/download/download_item_model.h"
 
+#include <vector>
+
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "base/string16.h"
+#include "base/string_util.h"
 #include "base/utf_string_conversions.h"
 #include "content/test/mock_download_item.h"
+#include "grit/generated_resources.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "grit/generated_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/text/bytes_formatting.h"
+#include "ui/gfx/font.h"
 
 using ::testing::AtLeast;
 using ::testing::Mock;
@@ -26,7 +30,17 @@ using ::testing::SetArgPointee;
 using ::testing::StrictMock;
 using ::testing::_;
 
-content::DownloadId::Domain kValidIdDomain = "valid DownloadId::Domain";
+namespace {
+
+// All the interrupt reasons we know of. The reason codes are unfortunately
+// sparse, making this array necessary.
+content::DownloadInterruptReason kInterruptReasons[] = {
+  content::DOWNLOAD_INTERRUPT_REASON_NONE,
+#define INTERRUPT_REASON(name,value)            \
+  content::DOWNLOAD_INTERRUPT_REASON_##name,
+#include "content/public/browser/download_interrupt_reason_values.h"
+#undef INTERRUPT_REASON
+};
 
 class DownloadItemModelTest : public testing::Test {
  public:
@@ -36,35 +50,40 @@ class DownloadItemModelTest : public testing::Test {
   }
 
  protected:
-  void SetupDownloadItemDefaults() {
+  void SetupDownloadItemDefaults(const GURL& url) {
     ON_CALL(item_, GetReceivedBytes()).WillByDefault(Return(1024));
     ON_CALL(item_, GetTotalBytes()).WillByDefault(Return(2048));
     ON_CALL(item_, IsInProgress()).WillByDefault(Return(false));
     ON_CALL(item_, TimeRemaining(_)).WillByDefault(Return(false));
-    ON_CALL(item_, GetState()).
-        WillByDefault(Return(content::DownloadItem::INTERRUPTED));
-    ON_CALL(item_, PromptUserForSaveLocation()).
-        WillByDefault(Return(false));
+    ON_CALL(item_, PromptUserForSaveLocation())
+        .WillByDefault(Return(false));
     ON_CALL(item_, GetMimeType()).WillByDefault(Return("text/html"));
     ON_CALL(item_, AllDataSaved()).WillByDefault(Return(false));
     ON_CALL(item_, GetOpenWhenComplete()).WillByDefault(Return(false));
     ON_CALL(item_, GetFileExternallyRemoved()).WillByDefault(Return(false));
+    ON_CALL(item_, GetState())
+        .WillByDefault(Return(content::DownloadItem::IN_PROGRESS));
+    ON_CALL(item_, GetURL()).WillByDefault(ReturnRefOfCopy(url));
+    ON_CALL(item_, GetFileNameToReportUser())
+        .WillByDefault(Return(FilePath(FILE_PATH_LITERAL("foo.bar"))));
   }
 
-  void SetupDownloadItem(const GURL& url,
-                         content::DownloadInterruptReason reason) {
+  void SetupInterruptedDownloadItem(content::DownloadInterruptReason reason) {
+    EXPECT_CALL(item_, GetLastReason()).WillRepeatedly(Return(reason));
+    EXPECT_CALL(item_, GetState())
+        .WillRepeatedly(Return(
+            (reason == content::DOWNLOAD_INTERRUPT_REASON_NONE) ?
+                content::DownloadItem::IN_PROGRESS :
+                content::DownloadItem::INTERRUPTED));
     model_.reset(new DownloadItemModel(&item_));
-    EXPECT_CALL(item_, GetURL()).WillRepeatedly(ReturnRefOfCopy(url));
-    EXPECT_CALL(item_, GetLastReason()).WillOnce(Return(reason));
   }
 
-  void TestDownloadItemModelStatusText(
-      const GURL& url, content::DownloadInterruptReason reason) {
-    SetupDownloadItem(url, reason);
+  void TestStatusText(
+      content::DownloadInterruptReason reason) {
+    SetupInterruptedDownloadItem(reason);
 
     int64 size = item_.GetReceivedBytes();
     int64 total = item_.GetTotalBytes();
-    bool know_size = (total > 0);
 
     ui::DataUnits amount_units = ui::GetByteDisplayUnits(total);
     string16 simple_size = ui::FormatBytesWithUnits(size, amount_units, false);
@@ -72,38 +91,52 @@ class DownloadItemModelTest : public testing::Test {
         ui::FormatBytesWithUnits(total, amount_units, true));
 
     string16 status_text;
-    string16 size_text;
-
-    status_text = DownloadItemModel::InterruptReasonStatusMessage(reason);
-
-    if (know_size) {
-      size_text = l10n_util::GetStringFUTF16(IDS_DOWNLOAD_RECEIVED_SIZE,
-                                             simple_size,
-                                             simple_total);
+    if (reason != content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED) {
+      string16 size_text = l10n_util::GetStringFUTF16(
+          IDS_DOWNLOAD_RECEIVED_SIZE, simple_size, simple_total);
+      status_text = size_text;
+      if (reason != content::DOWNLOAD_INTERRUPT_REASON_NONE) {
+        status_text = status_text + ASCIIToUTF16(" ") +
+            DownloadItemModel::InterruptReasonStatusMessage(reason);
+      }
     } else {
-      size_text = ui::FormatBytes(size);
+      status_text = DownloadItemModel::InterruptReasonStatusMessage(reason);
     }
-    size_text = size_text + ASCIIToUTF16(" ");
-    if (reason != content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED)
-      status_text = size_text + status_text;
 
-    DVLOG(2) << " " << __FUNCTION__ << "()" << " '" << status_text << "'";
+    DVLOG(1) << " " << __FUNCTION__ << "()" << " '" << status_text << "'";
 
     string16 text = model_->GetStatusText();
-
     EXPECT_EQ(status_text, text);
+    ::testing::Mock::VerifyAndClearExpectations(&item_);
   }
 
-  void TestDownloadItemModelStatusTextAllReasons(const GURL& url) {
-    SetupDownloadItemDefaults();
-
-#define INTERRUPT_REASON(name, value)  \
-    TestDownloadItemModelStatusText(url, \
-                                    content::DOWNLOAD_INTERRUPT_REASON_##name);
+  void TestTooltipWithFixedWidth(const gfx::Font& font, int width) {
+    content::DownloadInterruptReason reason = item_.GetLastReason();
+    string16 tooltip = model_->GetTooltipText(font, width);
+    std::vector<string16> lines;
+
+    DVLOG(1) << "Tooltip: '" << tooltip << "'";
+    Tokenize(tooltip, ASCIIToUTF16("\n"), &lines);
+    for (unsigned i = 0; i < lines.size(); ++i)
+      EXPECT_GE(width, font.GetStringWidth(lines[i]));
+    if (reason == content::DOWNLOAD_INTERRUPT_REASON_NONE ||
+        reason == content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED) {
+      // Only expect the filename for non-interrupted and canceled downloads.
+      EXPECT_EQ(1u, lines.size());
+    } else {
+      EXPECT_EQ(2u, lines.size());
+    }
+  }
 
-  #include "content/public/browser/download_interrupt_reason_values.h"
+  void TestTooltipText(content::DownloadInterruptReason reason) {
+    const int kSmallTooltipWidth = 40;
+    const int kLargeTooltipWidth = 1000;
+    gfx::Font font;
 
-#undef INTERRUPT_REASON
+    SetupInterruptedDownloadItem(reason);
+    TestTooltipWithFixedWidth(font, kSmallTooltipWidth);
+    TestTooltipWithFixedWidth(font, kLargeTooltipWidth);
+    ::testing::Mock::VerifyAndClearExpectations(&item_);
   }
 
  private:
@@ -112,7 +145,22 @@ class DownloadItemModelTest : public testing::Test {
   NiceMock<content::MockDownloadItem> item_;
 };
 
+}  // namespace
+
 // Test download error messages.
-TEST_F(DownloadItemModelTest, Interrupts) {
-  TestDownloadItemModelStatusTextAllReasons(GURL("http://foo.bar/"));
+TEST_F(DownloadItemModelTest, InterruptStatus) {
+  SetupDownloadItemDefaults(GURL("http://foo.bar/"));
+  // We are iterating through all the reason codes to make sure all message
+  // strings have the correct format placeholders. If not, GetStringFUTF16()
+  // will throw up.
+  for (unsigned i = 0; i < arraysize(kInterruptReasons); ++i)
+    TestStatusText(kInterruptReasons[i]);
+}
+
+TEST_F(DownloadItemModelTest, InterruptTooltip) {
+  SetupDownloadItemDefaults(GURL("http://foo.bar/"));
+  // Iterating through all the reason codes to exercise all the status strings.
+  // We also check whether the strings are properly elided.
+  for (unsigned i = 0; i < arraysize(kInterruptReasons); ++i)
+    TestTooltipText(kInterruptReasons[i]);
 }

chrome/browser/ui/cocoa/download/download_item_controller.mm

@@ -245,9 +245,9 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
 }
 
 - (void)updateToolTip {
-  string16 elidedFilename = ui::ElideFilename(
-      [self download]->GetFileNameToReportUser(), *font_, kToolTipMaxWidth);
-  [progressView_ setToolTip:base::SysUTF16ToNSString(elidedFilename)];
+  string16 tooltip_text =
+      bridge_->download_model()->GetTooltipText(*font_, kToolTipMaxWidth);
+  [progressView_ setToolTip:base::SysUTF16ToNSString(tooltip_text)];
 }
 
 - (void)clearDangerousMode {

chrome/browser/ui/cocoa/download/download_item_mac.mm

@@ -57,10 +57,13 @@ void DownloadItemMac::OnDownloadUpdated(content::DownloadItem* download) {
       download_util::NotifySystemOfDownloadComplete(download->GetFullPath());
       // fall through
     case DownloadItem::IN_PROGRESS:
-    case DownloadItem::INTERRUPTED:
     case DownloadItem::CANCELLED:
       [item_controller_ setStateFromDownload:download_model_.get()];
       break;
+    case DownloadItem::INTERRUPTED:
+      [item_controller_ updateToolTip];
+      [item_controller_ setStateFromDownload:download_model_.get()];
+      break;
     default:
       NOTREACHED();
   }

chrome/browser/ui/gtk/download/download_item_gtk.cc

@@ -335,6 +335,7 @@ void DownloadItemGtk::OnDownloadUpdated(DownloadItem* download) {
       break;
     case DownloadItem::INTERRUPTED:
       StopDownloadProgress();
+      UpdateTooltip();
 
       complete_animation_.Show();
       break;
@@ -476,11 +477,9 @@ void DownloadItemGtk::LoadIcon() {
 }
 
 void DownloadItemGtk::UpdateTooltip() {
-  string16 elided_filename = ui::ElideFilename(
-      get_download()->GetFileNameToReportUser(),
-      gfx::Font(), kTooltipMaxWidth);
-  gtk_widget_set_tooltip_text(body_.get(),
-                              UTF16ToUTF8(elided_filename).c_str());
+  string16 tooltip_text =
+      download_model_->GetTooltipText(gfx::Font(), kTooltipMaxWidth);
+  gtk_widget_set_tooltip_text(body_.get(), UTF16ToUTF8(tooltip_text).c_str());
 }
 
 void DownloadItemGtk::UpdateNameLabel() {

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

@@ -49,6 +49,7 @@ static const int kHorizontalTextPadding = 2;  // Pixels
 static const int kVerticalPadding = 3;        // Pixels
 static const int kVerticalTextSpacer = 2;     // Pixels
 static const int kVerticalTextPadding = 2;    // Pixels
+static const int kTooltipMaxWidth = 800;      // Pixels
 
 // We add some padding before the left image so that the progress animation icon
 // hides the corners of the left image.
@@ -184,9 +185,6 @@ DownloadItemView::DownloadItemView(DownloadItem* download,
 
   LoadIcon();
 
-  // Initial tooltip value.
-  tooltip_text_ = download_->GetFileNameToReportUser().LossyDisplayName();
-
   font_ = rb.GetFont(ui::ResourceBundle::BaseFont);
   box_height_ = std::max<int>(2 * kVerticalPadding + font_.GetHeight() +
                                   kVerticalTextPadding + font_.GetHeight(),
@@ -204,6 +202,8 @@ DownloadItemView::DownloadItemView(DownloadItem* download,
 
   UpdateDropDownButtonPosition();
 
+  tooltip_text_ = model_->GetTooltipText(font_, kTooltipMaxWidth);
+
   if (model_->IsDangerous())
     ShowWarningDialog();
 
@@ -254,20 +254,6 @@ void DownloadItemView::OnExtractIconComplete(IconManager::Handle handle,
 void DownloadItemView::OnDownloadUpdated(DownloadItem* download) {
   DCHECK(download == download_);
 
-  string16 old_tip = tooltip_text_;
-  content::DownloadInterruptReason reason = download_->GetLastReason();
-
-  if ((download_->GetState() == DownloadItem::INTERRUPTED) &&
-      (reason != content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED)) {
-    // Use two lines: The file name, and the message.
-    tooltip_text_ = download_->GetFileNameToReportUser().LossyDisplayName();
-    tooltip_text_ += ASCIIToUTF16("\n");
-    // The message is localized.
-    tooltip_text_ += DownloadItemModel::InterruptReasonMessage(reason);
-  } else {
-    tooltip_text_ = download_->GetFileNameToReportUser().LossyDisplayName();
-  }
-
   if (IsShowingWarningDialog() && !model_->IsDangerous()) {
     // We have been approved.
     ClearWarningDialog();
@@ -319,8 +305,11 @@ void DownloadItemView::OnDownloadUpdated(DownloadItem* download) {
     status_text_ = status_text;
   }
 
-  if (old_tip != tooltip_text_)
+  string16 new_tip = model_->GetTooltipText(font_, kTooltipMaxWidth);
+  if (new_tip != tooltip_text_) {
+    tooltip_text_ = new_tip;
     TooltipTextChanged();
+  }
 
   UpdateAccessibleName();