Fix SizeLabelToMinWidth() function such that no unnecessary space

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

 // Copyright 2015 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.
 //
 // A view that implements one download on the Download shelf.
 // Each DownloadItemView contains an application icon, a text label
 // indicating the download's file name, a text label indicating the
 // download's status (such as the number of bytes downloaded so far)
 // and a button for canceling an in progress download, or opening
 // the completed download.
@@ skipping 118 matching lines @@
   // gfx::AnimationDelegate implementation.
   void AnimationProgressed(const gfx::Animation* animation) override;
 
  protected:
   // Overridden from views::View:
   void OnPaint(gfx::Canvas* canvas) override;
   void OnFocus() override;
   void OnBlur() override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(DownloadItemViewDangerousDownloadLabelTest,
+                           AdjustTextAndGetSize_VeryShortText);
+  FRIEND_TEST_ALL_PREFIXES(DownloadItemViewDangerousDownloadLabelTest,
+                           AdjustTextAndGetSize_MoreTextOnFirstLine);
+  FRIEND_TEST_ALL_PREFIXES(DownloadItemViewDangerousDownloadLabelTest,
+                           AdjustTextAndGetSize_VeryLongTextWithoutSpace);

[Peter Kasting, 2016/12/08 21:53:52]

Nit: No strong feeling here, but if you had a single testcase, it would reduce
the number of declarations here from three to one, and eliminate the need to
repeatedly call SetMultiLine(true) in that function (because I assume you'd just
use one Label, and SetText() it repeatedly).

[Jialiu Lin, 2016/12/08 22:44:03]

merged into a single test case.

+
   enum State { NORMAL = 0, HOT, PUSHED };
   class DropDownButton;
 
   enum Mode {
     NORMAL_MODE = 0,  // Showing download item.
     DANGEROUS_MODE,   // Displaying the dangerous download warning.
     MALICIOUS_MODE    // Displaying the malicious download warning.
   };
 
   void OpenDownload();
@@ skipping 41 matching lines @@
   bool IsShowingWarningDialog() const {
     return mode_ == DANGEROUS_MODE || mode_ == MALICIOUS_MODE;
   }
 
   // Clears or shows the warning dialog as per the state of |model_|.
   void ToggleWarningDialog();
 
   // Reverts from dangerous mode to normal download mode.
   void ClearWarningDialog();
 
-  // Start displaying the dangerous download warning or the malicious download
+  // Starts displaying the dangerous download warning or the malicious download
   // warning.
   void ShowWarningDialog();
 
   // Returns the current warning icon (should only be called when the view is
   // actually showing a warning).
   gfx::ImageSkia GetWarningIcon();
 
   // Sets |size| with the size of the Save and Discard buttons (they have the
   // same size).
   gfx::Size GetButtonSize() const;
 
   // Sizes the dangerous download label to a minimum width available using 2
   // lines.  The size is computed only the first time this method is invoked
   // and simply returned on subsequent calls.
   void SizeLabelToMinWidth();
 
+  // Called by SizeLabelToMinWidth() to compute the optimal size (a.k.a minimum
+  // width) of a label and set its text accordingly.

[Peter Kasting, 2016/12/08 21:53:52]

Nit: Maybe more detail would help, especially for people trying to read the test
and see why it's testing what it is:

Given a multiline |label|, decides whether it should be displayed on one line
(if short), or broken across two lines.  In the latter case, linebreaks near the
middle of the string and sets the label's text accordingly.  Returns the
preferred size for the label.

[Jialiu Lin, 2016/12/08 22:44:03]

Done.

+  static gfx::Size AdjustTextAndGetSize(views::Label* label);
+
   // Reenables the item after it has been disabled when a user clicked it to
   // open the downloaded file.
   void Reenable();
 
   // Releases drop down button after showing a context menu.
   void ReleaseDropdown();
 
   // Update the accessible name to reflect the current state of the control,
   // so that screenreaders can access the filename, status text, and
   // dangerous download warning message (if any).
@@ skipping 102 matching lines @@
   std::unique_ptr<extensions::ExperienceSamplingEvent> sampling_event_;
 
   // Method factory used to delay reenabling of the item when opening the
   // downloaded file.
   base::WeakPtrFactory<DownloadItemView> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItemView);
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_DOWNLOAD_DOWNLOAD_ITEM_VIEW_H_