Remove unused custom vertical text alignment in CanvasSkia.

ui/gfx/canvas_skia.cc

 // 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.
 
 #include "ui/gfx/canvas.h"
 
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/base/range/range.h"
@@ skipping 62 matching lines @@
       *bitmap.getAddr32(x, y - 1) != halo_color &&
       *bitmap.getAddr32(x, y - 1) != 0)
     return true;  // Touched pixel above.
   if (y < bitmap.height() - 1 &&
       *bitmap.getAddr32(x, y + 1) != halo_color &&
       *bitmap.getAddr32(x, y + 1) != 0)
     return true;  // Touched pixel below.
   return false;
 }
 
-// Apply vertical alignment per |flags|. Returns y-coordinate delta.
-int VAlignText(int text_height,
-               int flags,
-               int available_height) {
-  if (flags & gfx::Canvas::TEXT_VALIGN_TOP)
-    return 0;
-
-  if (flags & gfx::Canvas::TEXT_VALIGN_BOTTOM)
-    return available_height - text_height;
-
-  // Default case: TEXT_VALIGN_MIDDLE.
-  return (available_height - text_height) / 2;
-}
-
 // Strips accelerator character prefixes in |text| if needed, based on |flags|.
 // Returns a range in |text| to underline or ui::Range::InvalidRange() if
 // underlining is not needed.
 ui::Range StripAcceleratorChars(int flags, string16* text) {
   if (flags & (gfx::Canvas::SHOW_PREFIX | gfx::Canvas::HIDE_PREFIX)) {
     int char_pos = -1;
     int char_span = 0;
     *text = gfx::RemoveAcceleratorChar(*text, '&', &char_pos, &char_span);
     if ((flags & gfx::Canvas::SHOW_PREFIX) && char_pos != -1)
       return ui::Range(char_pos, char_pos + char_span);
@@ skipping 148 matching lines @@
                                    const gfx::Font& font,
                                    SkColor color,
                                    const gfx::Rect& text_bounds,
                                    int flags,
                                    const ShadowValues& shadows) {
   if (!IntersectsClipRect(text_bounds))
     return;
 
   flags = AdjustPlatformSpecificFlags(text, flags);
 
-#if defined(OS_WIN)
-  // TODO(asvitkine): On Windows, MULTI_LINE implies top alignment.
-  //                  http://crbug.com/107357

[Alexei Svitkine (slow), 2012/11/05 15:44:42]

Last time I attempted to remove this block, it caused a regression with the SSL
settings bubble on Windows with the paragraph that described the current
connection (I believe that bubble has now been changed to have that paragraph in
a 2nd tab inside the bubble).

Can you verify that your ifdef added below doesn't cause a regression? (From
reading the code, it looks correct - so just pointing out a way to test this, if
you haven't seen already.)

[msw, 2012/11/05 22:40:50]

The website settings bubble's connection tab doesn't change on Win/CrOS.

-  if (flags & MULTI_LINE) {
-    flags &= ~(TEXT_VALIGN_MIDDLE | TEXT_VALIGN_BOTTOM);
-    flags |= TEXT_VALIGN_TOP;
-  }
-#endif
-
   gfx::Rect clip_rect(text_bounds);
   clip_rect.Inset(ShadowValue::GetMargin(shadows));
 
   canvas_->save(SkCanvas::kClip_SaveFlag);
   ClipRect(clip_rect);
 
   gfx::Rect rect(text_bounds);
   string16 adjusted_text = text;
 
 #if defined(OS_WIN)
@@ skipping 13 matching lines @@
     std::vector<string16> strings;
     ui::ElideRectangleText(adjusted_text,
                            font,
                            text_bounds.width(), text_bounds.height(),
                            wrap_behavior,
                            &strings);
 
     for (size_t i = 0; i < strings.size(); i++) {
       ui::Range range = StripAcceleratorChars(flags, &strings[i]);
       UpdateRenderText(rect, strings[i], font, flags, color, render_text.get());
+      const int line_height = render_text->GetStringSize().height();
 
-      // Apply vertical alignment over the block of text using the height of the
-      // first line. This may not be correct if different lines in the text have
-      // different heights, but avoids needing to do two passes.
-      const int line_height = render_text->GetStringSize().height();
+      // TODO(msw|asvitkine): Center Windows multi-line text: crbug.com/107357
+#if !defined(OS_WIN)
       if (i == 0) {
-        rect.Offset(0, VAlignText(strings.size() * line_height,
-                                  flags,
-                                  text_bounds.height()));
+        // TODO(msw|asvitkine): Support multi-line text with varied heights.
+        const int aggregate_height = strings.size() * line_height;
+        rect.Offset(0, (text_bounds.height() - aggregate_height) / 2);
       }
+#endif
+
       rect.set_height(line_height);
 
       ApplyUnderlineStyle(range, render_text.get());
       render_text->SetDisplayRect(rect);
       render_text->Draw(this);
       rect.Offset(0, line_height);
     }
   } else {
     ui::Range range = StripAcceleratorChars(flags, &adjusted_text);
     bool elide_text = ((flags & NO_ELLIPSIS) == 0);
@@ skipping 14 matching lines @@
       ElideTextAndAdjustRange(font,
                               text_bounds.width(),
                               &adjusted_text,
                               &range);
     }
 
     UpdateRenderText(rect, adjusted_text, font, flags, color,
                      render_text.get());
 
     const int line_height = render_text->GetStringSize().height();
-    rect.Offset(0, VAlignText(line_height, flags, text_bounds.height()));
+    // Center the text vertically.
+    rect.Offset(0, (text_bounds.height() - line_height) / 2);
     rect.set_height(line_height);
     render_text->SetDisplayRect(rect);
 
     ApplyUnderlineStyle(range, render_text.get());
     render_text->Draw(this);
   }
 
   canvas_->restore();
 }
 
@@ skipping 96 matching lines @@
       break;
   }
 
   // Default to left alignment unless right alignment was chosen above.
   if (!(flags & TEXT_ALIGN_RIGHT))
     flags |= TEXT_ALIGN_LEFT;
 
   gfx::Rect rect = display_rect;
   UpdateRenderText(rect, clipped_text, font, flags, color, render_text.get());
 
   const int line_height = render_text->GetStringSize().height();

[Alexei Svitkine (slow), 2012/11/05 15:44:42]

Nit: This block is the same as above. Can you make a helper function?

[msw, 2012/11/05 22:40:50]

I'd rather follow up with bigger vertical placement changes.

-  rect.Offset(0, VAlignText(line_height, flags, display_rect.height()));
+  // Center the text vertically.
+  rect.Offset(0, (display_rect.height() - line_height) / 2);
   rect.set_height(line_height);
   render_text->SetDisplayRect(rect);
 
   canvas_->save(SkCanvas::kClip_SaveFlag);
   ClipRect(display_rect);
   render_text->Draw(this);
   canvas_->restore();
 }
 
 }  // namespace gfx