Change OmniboxPopupTruncatingLabel to be based on UILabel.

ios/chrome/browser/ui/omnibox/truncating_attributed_label.mm

Index: ios/chrome/browser/ui/omnibox/truncating_attributed_label.mm
diff --git a/ios/chrome/browser/ui/omnibox/truncating_attributed_label.mm b/ios/chrome/browser/ui/omnibox/truncating_attributed_label.mm
index c4a13e49895e5c9e4f4fb2d622700fc8b1bbc867..81b1bc83b50e9cbc5d0c6055f17d8b4d053c734f 100644
--- a/ios/chrome/browser/ui/omnibox/truncating_attributed_label.mm
+++ b/ios/chrome/browser/ui/omnibox/truncating_attributed_label.mm
@@ -15,32 +15,14 @@
 @end
 
 @implementation OmniboxPopupTruncatingLabel {
-  // Attributed text.
-  base::scoped_nsobject<CATextLayer> textLayer_;
-  // Gradient used to create fade effect. Changes based on view.frame size.
-  base::scoped_nsobject<UIImage> gradient_;
-
   base::mac::ObjCPropertyReleaser propertyReleaser_OmniboxPopupTruncatingLabel_;
 }
 
 @synthesize truncateMode = truncateMode_;
-@synthesize attributedText = attributedText_;
-@synthesize highlighted = highlighted_;
-@synthesize highlightedText = highlightedText_;
-@synthesize textAlignment = textAlignment_;
 
 - (void)setup {
   self.backgroundColor = [UIColor clearColor];
-  self.contentMode = UIViewContentModeRedraw;
   truncateMode_ = OmniboxPopupTruncatingTail;
-
-  // Disable animations in CATextLayer.
-  textLayer_.reset([[CATextLayer layer] retain]);
-  base::scoped_nsobject<NSDictionary> actions([[NSDictionary alloc]
-      initWithObjectsAndKeys:[NSNull null], @"contents", nil]);
-  [textLayer_ setActions:actions];
-  [textLayer_ setFrame:self.bounds];
-  [textLayer_ setContentsScale:[[UIScreen mainScreen] scale]];
 }
 
 - (id)initWithFrame:(CGRect)frame {
@@ -48,6 +30,7 @@
   if (self) {
     propertyReleaser_OmniboxPopupTruncatingLabel_.Init(
         self, [OmniboxPopupTruncatingLabel class]);
+    self.lineBreakMode = NSLineBreakByClipping;
     [self setup];
   }
   return self;
@@ -58,47 +41,62 @@
   [self setup];
 }
 
-- (void)setFrame:(CGRect)frame {
-  [super setFrame:frame];
-  [textLayer_ setFrame:self.bounds];
-
-  // Cache the fade gradient when the frame changes.
-  if (!CGRectIsEmpty(frame) &&
-      (!gradient_.get() || !CGSizeEqualToSize([gradient_ size], frame.size))) {
-    CGRect rect = CGRectMake(0, 0, frame.size.width, frame.size.height);
-    gradient_.reset([[self getLinearGradient:rect] retain]);
-  }
-}
-
-- (void)drawRect:(CGRect)rect {
-  if ([attributedText_ length] == 0)
-    return;
-
+// Draw fade gradient mask if text is wider than rect.
+- (void)drawTextInRect:(CGRect)requestedRect {
   CGContextRef context = UIGraphicsGetCurrentContext();
   CGContextSaveGState(context);
-  [textLayer_ setString:highlighted_ ? highlightedText_ : attributedText_];
-  CGContextClipToMask(context, self.bounds, [gradient_ CGImage]);
-  [textLayer_ renderInContext:context];
 
-  CGContextRestoreGState(context);
-}
+  CGSize size = CGSizeZero;
+  if (self.attributedText) {
+    size = [self.attributedText size];
+  } else if (self.font) {
+    size = [self.text sizeWithAttributes:@{NSFontAttributeName : self.font}];
+    // sizeWithAttributes: may return fractional values, so ceil the width and
+    // height to preserve the behavior of sizeWithFont:.
+    size = CGSizeMake(ceil(size.width), ceil(size.height));
+  }
 
-- (void)setTextAlignment:(NSTextAlignment)textAlignment {
-  if (textAlignment == NSTextAlignmentLeft) {
-    [textLayer_ setAlignmentMode:kCAAlignmentLeft];
-    self.truncateMode = OmniboxPopupTruncatingTail;
-  } else if (textAlignment == NSTextAlignmentRight) {
-    [textLayer_ setAlignmentMode:kCAAlignmentRight];
-    self.truncateMode = OmniboxPopupTruncatingHead;
-  } else if (textAlignment == NSTextAlignmentNatural) {
-    [textLayer_ setAlignmentMode:kCAAlignmentNatural];
-    self.truncateMode = OmniboxPopupTruncatingTail;
-  } else {
-    NOTREACHED();
+  if (size.width > requestedRect.size.width) {
+    UIImage* image = [self getLinearGradient:requestedRect];

[justincohen, 2017/02/20 20:07:24]

Does it matter this isn't cached like it used to be?

[Justin Donnelly, 2017/02/21 18:16:50]

I don't think so. GTMFadeTruncatingLabel doesn't cache it nor does the omnibox
text field when it handles the clipping itself:

https://cs.chromium.org/chromium/src/ios/chrome/browser/ui/omnibox/omnibox_te...

[justincohen, 2017/02/22 03:52:29]

I had to go back to 2012 ( https://b.corp.google.com/issues/5954680 ) to find
where we added this caching.  Do you think (four years later!) maybe
getLinearGradient is faster, or drawTextInRect is called less?

[Justin Donnelly, 2017/02/22 17:17:42]

I don't honestly know. Without the caching, there's no jank in the simulator but
I don't have an old device to test it on to see if there's a performance
regression. I've restored the caching in the latest patch set, lmkwyt.

(The latest patch set also restores the handling of text alignment which I
mistakenly removed. And I actually tested this in an RTL language this time to
verify it's working correctly.)

+    CGContextClipToMask(context, self.bounds, image.CGImage);
   }
-  if (textAlignment != textAlignment_)
-    gradient_.reset();
-  textAlignment_ = textAlignment;
+
+  if (self.attributedText) {
+    NSMutableAttributedString* attributedString =
+        [[self.attributedText mutableCopy] autorelease];
+
+    NSMutableParagraphStyle* textStyle =
+        [[[NSParagraphStyle defaultParagraphStyle] mutableCopy] autorelease];
+    textStyle.lineBreakMode = self.lineBreakMode;
+    textStyle.alignment = self.textAlignment;
+    [attributedString addAttribute:NSParagraphStyleAttributeName
+                             value:textStyle
+                             range:NSMakeRange(0, [self.text length])];
+
+    [attributedString drawInRect:requestedRect];

[justincohen, 2017/02/20 20:07:24]

Is lineBreakMode and textAlignment different in self.attributedText?   Why is
this copy necessary?

[Justin Donnelly, 2017/02/21 18:16:50]

No, they behave the same. Maybe it would be better to just hardcode the line
break mode here to NSLineBreakByClipping (since the other ones probably don't
make sense with fading) and to have the client specify the alignment as part of
their attributed string. I was trying to stay consistent with the .text case and
how GTMFadeTruncatingLabel currently works, but I could go either way. Let me
know what you think.
The copy is necessary because self.attributedText is immutable.

[justincohen, 2017/02/22 03:52:29]

What's the default linebreakmode?  If the client specifies everything in their
attributed string is any of this necessary?

I don't feel strongly either way -- this is fine too.

[Justin Donnelly, 2017/02/22 17:17:42]

The default is NSLineBreakByTruncatingTail (trailing ellipsis). Yes, we could
have the client specify these properties instead. But if you don't feel
strongly, I'd prefer to keep it this way to make it easier to switch to
GTMFadeTruncatingLabel in the future.

+  } else if (self.font) {
+    // The UILabel docs say the default textColor is black and experimentation
+    // shows that calling -textColor will return the cached [UIColor blackColor]
+    // when called on a freshly alloc/init-ed UILabel, or a UILabel whose
+    // textColor has been set to nil.
+    //
+    // @see
+    // https://developer.apple.com/Library/ios/documentation/UIKit/Reference/UILabel_Class/Reference/UILabel.html#//apple_ref/occ/instp/UILabel/textColor
+    // (NOTE(bgoodwin): interesting side-note. These docs also say setting
+    // textColor to nil will result in an exception. In my testing, that did not
+    // happen.)
+    NSMutableParagraphStyle* textStyle =
+        [[[NSParagraphStyle defaultParagraphStyle] mutableCopy] autorelease];
+    textStyle.lineBreakMode = self.lineBreakMode;
+    textStyle.alignment = self.textAlignment;
+    NSDictionary* attributes = @{
+      NSFontAttributeName : self.font,
+      NSParagraphStyleAttributeName : textStyle,
+      NSForegroundColorAttributeName : self.textColor
+    };
+    [self.text drawInRect:requestedRect withAttributes:attributes];

[justincohen, 2017/02/20 20:07:24]

Why not just create an attributed string from text and hit the same path as
above?

[Justin Donnelly, 2017/02/21 18:16:50]

It seemed simpler and safer to me to just copy the code from
GTMFadeTruncatingLabel and match the existing behavior for self.text.

But on second thought, this case is dead code. The only use of this class is in
OmniboxPopupMaterialRow and OmniboxPopupMaterialViewController which use
attributedText. I've removed it. Now this class can just be the attributedText
version of GTMFadeTruncatingLabel until such time as I can get
GTMFadeTruncatingLabel to handle both cases.

+  }
+  CGContextRestoreGState(context);
 }
 
 // Create gradient opacity mask based on direction.