Fix text wrapping problem in the download_item_controller on Yosemite.

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

 // 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.
 
 #import "chrome/browser/ui/cocoa/download/download_item_controller.h"
 
 #include "base/mac/bundle_locations.h"
+#include "base/mac/mac_util.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_item_model.h"
 #include "chrome/browser/download/download_shelf_context_menu.h"
 #include "chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h"
 #import "chrome/browser/themes/theme_properties.h"
@@ skipping 104 matching lines @@
   [self updateExperienceSamplingEvent:ExperienceSamplingEvent::kIgnore];
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [progressView_ setController:nil];
   [[self view] removeFromSuperview];
   [super dealloc];
 }
 
 - (void)awakeFromNib {
   [progressView_ setController:self];
 
+  // The new system font on Yosemite brings slightly different font metrics.
+  // The |dangerousDownloadLabel_| is 22 points high with two lines of text
+  // in the nib, but on Yosemite it needs 24 points to hold that same text.
+  // The best alternative to hard coding the 2.0pt delta is to have the
+  // textfield compute the height necessary to display two lines of text.
+  // In order to disturb as little as possible, the code below only runs on
+  // Yosemite, and restores the original state of |dangerousDownloadLabel_|
+  // after the computation (except for, of course, its height).

[asanka, 2015/02/21 02:14:53]

Comment is out of date?

[shrike, 2015/02/23 20:12:14]

It's not, but I rewrote it in the new cl.

+  // http://crbug.com/454782 .
+  if (base::mac::IsOSYosemiteOrLater()) {
+    NSString* placeholderText = [dangerousDownloadLabel_ stringValue];

[asanka, 2015/02/21 02:14:54]

Let's move this height adjustment logic into -showDangerousWarning:

+    NSRect labelFrame = [dangerousDownloadLabel_ frame];
+    [dangerousDownloadLabel_ setStringValue:@"Two lines\nof text"];

[asanka, 2015/02/21 02:14:54]

Once this is moved to -showDangerousWarning:, we could use something like
[NSString stringWithFormat:@"%@\n%@", dangerousWarning, dangerousWarning] to get
the height from the actual text we want to display, in case that makes a
difference.

[shrike, 2015/02/23 20:12:14]

The idea behind this change is just to quickly fix up the textfield's height on
nib load so that the textfield works as expected thereafter. The textfield
already contains placeholder text that wraps to two lines in the nib, but I
couldn't think of a way to get sizeToFit to use that placeholder text to compute
the two line height (the Appkit wanted to lay it out on a single line). That's
why I replaced it with a temp string that forced wrapping (ugly, but as a I
said, I couldn't come up with another way to do it).

I have since reworked this code to use the existing placeholder string to
generate the two-line height. I think it's cleaner/simpler to make this little
adjustment once at nib load time rather than every time showDangerousWarning:
gets called. Additionally, this adjustment is really independent of whatever
string comes into showDangerousWarning:.

Finally, per thakis's recommendation, I have removed the yosemite check.

[asanka, 2015/02/24 03:16:13]

showDangerousWarning is called at most once per download. It's a constraint of
the downloads system that a download only transitions once from "safe" to
"dangerous."

Why do you think this adjustment is going to be independent of the string that
should be displayed?

Given that we aren't making sweeping changes elsewhere in Chrome to accommodate
this change in Yosemite, should we consider changing the XIB and avoid runtime
changes?

+    [dangerousDownloadLabel_ sizeToFit];
+    CGFloat heightDelta = [dangerousDownloadLabel_ frame].size.height -
+        labelFrame.size.height;
+    labelFrame.origin.y -= heightDelta / 2.0;
+    labelFrame.size.height += heightDelta;
+    [dangerousDownloadLabel_ setFrame:labelFrame];
+    [dangerousDownloadLabel_ setStringValue:placeholderText];
+  }
   GTMUILocalizerAndLayoutTweaker* localizerAndLayoutTweaker =
       [[[GTMUILocalizerAndLayoutTweaker alloc] init] autorelease];
   [localizerAndLayoutTweaker applyLocalizer:localizer_ tweakingUI:[self view]];
 
   [self setStateFromDownload:bridge_->download_model()];
 
   bridge_->LoadIcon();
   [self updateToolTip];
 }
 
@@ skipping 231 matching lines @@
   base::scoped_nsobject<DownloadShelfContextMenuController> menuController(
       [[DownloadShelfContextMenuController alloc]
         initWithItemController:self
                   withDelegate:nil]);
   [NSMenu popUpContextMenu:[menuController menu]
                  withEvent:[NSApp currentEvent]
                    forView:[self view]];
 }
 
 @end