Remove the 0x0 canvas used for computing text sizes. Creating this is not necessary since the text can be sized using the desktop DC, which should be faster. I made this function static, and moved the functions it used into file-local functions since they didn't depend on the class either.

This is trying to fix bug 49. This code is very close to the crash, and a 0x0 canvas seems like it might cause problems. However, it is unclear whether this will have any effect (other than performance).
Review URL: http://codereview.chromium.org/10409

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

3 files changed, 90 insertions, 92 deletions

diff --git a/chrome/common/gfx/chrome_canvas.cc b/chrome/common/gfx/chrome_canvas.cc
index 3114b14..7c19b874 100644
--- a/chrome/common/gfx/chrome_canvas.cc
+++ b/chrome/common/gfx/chrome_canvas.cc
@@ -12,6 +12,84 @@
 #include "chrome/common/gfx/chrome_font.h"
 #include "chrome/common/l10n_util.h"
 
+namespace {
+
+// We make sure that LTR text we draw in an RTL context is modified
+// appropriately to make sure it maintains it LTR orientation.
+void DoDrawText(HDC hdc, const std::wstring& text,
+                RECT* text_bounds, int flags) {
+  std::wstring localized_text;
+  const wchar_t* string_ptr = text.c_str();
+  int string_size = static_cast<int>(text.length());
+  if (l10n_util::AdjustStringForLocaleDirection(text, &localized_text)) {
+    string_ptr = localized_text.c_str();
+    string_size = static_cast<int>(localized_text.length());
+  }
+
+  DrawText(hdc, string_ptr, string_size, text_bounds, flags);
+}
+
+// Compute the windows flags necessary to implement the provided text
+// ChromeCanvas flags.
+int ComputeFormatFlags(int flags) {
+  int f = 0;
+
+  // Setting the text alignment explicitly in case it hasn't already been set.
+  // This will make sure that we don't align text to the left on RTL locales
+  // just because no alignment flag was passed to DrawStringInt().
+  if (!(flags & (ChromeCanvas::TEXT_ALIGN_CENTER |
+                 ChromeCanvas::TEXT_ALIGN_RIGHT |
+                 ChromeCanvas::TEXT_ALIGN_LEFT))) {
+    flags |= l10n_util::DefaultCanvasTextAlignment();
+  }
+
+  if (flags & ChromeCanvas::HIDE_PREFIX)
+    f |= DT_HIDEPREFIX;
+  else if ((flags & ChromeCanvas::SHOW_PREFIX) == 0)
+    f |= DT_NOPREFIX;
+
+  if (flags & ChromeCanvas::MULTI_LINE) {
+    f |= DT_WORDBREAK;
+  } else {
+    f |= DT_SINGLELINE | DT_VCENTER;
+    if (!(flags & ChromeCanvas::NO_ELLIPSIS))
+      f |= DT_END_ELLIPSIS;
+  }
+
+  // vertical alignment
+  if (flags & ChromeCanvas::TEXT_VALIGN_TOP)
+    f |= DT_TOP;
+  else if (flags & ChromeCanvas::TEXT_VALIGN_BOTTOM)
+    f |= DT_BOTTOM;
+  else
+    f |= DT_VCENTER;
+
+  // horizontal alignment
+  if (flags & ChromeCanvas::TEXT_ALIGN_CENTER)
+    f |= DT_CENTER;
+  else if (flags & ChromeCanvas::TEXT_ALIGN_RIGHT)
+    f |= DT_RIGHT;
+  else
+    f |= DT_LEFT;
+
+  // In order to make sure RTL/BiDi strings are rendered correctly, we must
+  // pass the flag DT_RTLREADING to DrawText (when the locale's language is
+  // a right-to-left language) so that Windows does the right thing.
+  //
+  // In addition to correctly displaying text containing both RTL and LTR
+  // elements (for example, a string containing a telephone number within a
+  // sentence in Hebrew, or a sentence in Hebrew that contains a word in
+  // English) this flag also makes sure that if there is not enough space to
+  // display the entire string, the ellipsis is displayed on the left hand side
+  // of the truncated string and not on the right hand side.
+  if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT)
+    f |= DT_RTLREADING;
+
+  return f;
+}
+
+}  // namespace
+
 ChromeCanvas::ChromeCanvas(int width, int height, bool is_opaque)
     : gfx::PlatformCanvasWin(width, height, is_opaque) {
 }
@@ -217,66 +295,11 @@ void ChromeCanvas::DrawBitmapInt(const SkBitmap& bitmap, int src_x, int src_y,
   drawRect(dest_rect, p);
 }
 
-int ChromeCanvas::ComputeFormatFlags(int flags) {
-  int f = 0;
-
-  // Setting the text alignment explicitly in case it hasn't already been set.
-  // This will make sure that we don't align text to the left on RTL locales
-  // just because no alignment flag was passed to DrawStringInt().
-  if (!(flags & (TEXT_ALIGN_CENTER | TEXT_ALIGN_RIGHT | TEXT_ALIGN_LEFT))) {
-    flags |= l10n_util::DefaultCanvasTextAlignment();
-  }
-
-  if (flags & HIDE_PREFIX)
-    f |= DT_HIDEPREFIX;
-  else if ((flags & SHOW_PREFIX) == 0)
-    f |= DT_NOPREFIX;
-
-  if (flags & MULTI_LINE) {
-    f |= DT_WORDBREAK;
-  } else {
-    f |= DT_SINGLELINE | DT_VCENTER;
-    if (!(flags & NO_ELLIPSIS))
-      f |= DT_END_ELLIPSIS;
-  }
-
-  // vertical alignment
-  if (flags & TEXT_VALIGN_TOP)
-    f |= DT_TOP;
-  else if (flags & TEXT_VALIGN_BOTTOM)
-    f |= DT_BOTTOM;
-  else
-    f |= DT_VCENTER;
-
-  // horizontal alignment
-  if (flags & TEXT_ALIGN_CENTER)
-    f |= DT_CENTER;
-  else if (flags & TEXT_ALIGN_RIGHT)
-    f |= DT_RIGHT;
-  else
-    f |= DT_LEFT;
-
-  // In order to make sure RTL/BiDi strings are rendered correctly, we must
-  // pass the flag DT_RTLREADING to DrawText (when the locale's language is
-  // a right-to-left language) so that Windows does the right thing.
-  //
-  // In addition to correctly displaying text containing both RTL and LTR
-  // elements (for example, a string containing a telephone number within a
-  // sentence in Hebrew, or a sentence in Hebrew that contains a word in
-  // English) this flag also makes sure that if there is not enough space to
-  // display the entire string, the ellipsis is displayed on the left hand side
-  // of the truncated string and not on the right hand side.
-  if (l10n_util::GetTextDirection() == l10n_util::RIGHT_TO_LEFT) {
-    f |= DT_RTLREADING;
-  }
-
-  return f;
-}
-
+// static
 void ChromeCanvas::SizeStringInt(const std::wstring& text,
                                  const ChromeFont& font,
                                  int *width, int *height, int flags) {
-  HDC dc = beginPlatformPaint();
+  HDC dc = GetDC(NULL);
   HFONT old_font = static_cast<HFONT>(SelectObject(dc, font.hfont()));
   RECT b;
   b.left = 0;
@@ -288,13 +311,14 @@ void ChromeCanvas::SizeStringInt(const std::wstring& text,
   }
   b.bottom = *height;
   DoDrawText(dc, text, &b, ComputeFormatFlags(flags) | DT_CALCRECT);
-  endPlatformPaint();
 
   // Restore the old font. This way we don't have to worry if the caller
   // deletes the font and the DC lives longer.
   SelectObject(dc, old_font);
   *width = b.right;
   *height = b.bottom;
+
+  ReleaseDC(NULL, dc);
 }
 
 void ChromeCanvas::DrawStringInt(const std::wstring& text, HFONT font,
@@ -331,22 +355,6 @@ void ChromeCanvas::DrawStringInt(const std::wstring& text,
                 l10n_util::DefaultCanvasTextAlignment());
 }
 
-// We make sure that LTR text we draw in an RTL context is modified
-// appropriately to make sure it maintains it LTR orientation.
-void ChromeCanvas::DoDrawText(HDC hdc, const std::wstring& text,
-                              RECT* text_bounds, int flags) {
-  std::wstring localized_text;
-  const wchar_t* string_ptr = text.c_str();
-  int string_size = static_cast<int>(text.length());
-  if (l10n_util::AdjustStringForLocaleDirection(text, &localized_text)) {
-    string_ptr = localized_text.c_str();
-    string_size = static_cast<int>(localized_text.length());
-  }
-
-  DrawText(hdc, string_ptr, string_size, text_bounds, flags);
-}
-
-
 void ChromeCanvas::DrawStringInt(const std::wstring& text,
                                  const ChromeFont& font,
                                  const SkColor& color,
diff --git a/chrome/common/gfx/chrome_canvas.h b/chrome/common/gfx/chrome_canvas.h
index b48fe0d..ddc3f28 100644
--- a/chrome/common/gfx/chrome_canvas.h
+++ b/chrome/common/gfx/chrome_canvas.h
@@ -144,13 +144,6 @@ class ChromeCanvas : public gfx::PlatformCanvasWin {
   // Draws a dotted gray rectangle used for focus purposes.
   void DrawFocusRect(int x, int y, int width, int height);
 
-  // Compute the size required to draw some text with the provided font.
-  // Attempts to fit the text with the provided width and height. Increases
-  // height and then width as needed to make the text fit. This method
-  // supports multiple lines.
-  void SizeStringInt(const std::wstring& test, const ChromeFont& font,
-                     int *width, int* height, int flags);
-
   // Tiles the image in the specified region.
   void TileImageInt(const SkBitmap& bitmap, int x, int y, int w, int h,
                     SkPorterDuff::Mode mode);
@@ -162,6 +155,13 @@ class ChromeCanvas : public gfx::PlatformCanvasWin {
   // Extracts a bitmap from the contents of this canvas.
   SkBitmap ExtractBitmap();
 
+  // Compute the size required to draw some text with the provided font.
+  // Attempts to fit the text with the provided width and height. Increases
+  // height and then width as needed to make the text fit. This method
+  // supports multiple lines.
+  static void SizeStringInt(const std::wstring& test, const ChromeFont& font,
+                            int *width, int* height, int flags);
+
  private:
   // Draws text with the specified color, font and location. The text is
   // aligned to the left, vertically centered, clipped to the region. If the
@@ -170,15 +170,6 @@ class ChromeCanvas : public gfx::PlatformCanvasWin {
                      const SkColor& color, int x, int y, int w, int h,
                      int flags);
 
-  // Compute the windows flags necessary to implement the provided text
-  // ChromeCanvas flags
-  int ChromeCanvas::ComputeFormatFlags(int flags);
-
-  // A wrapper around Windows' DrawText. This function takes care of adding
-  // Unicode directionality marks to the text in certain cases.
-  void DoDrawText(HDC hdc, const std::wstring& text, RECT* text_bounds,
-                  int flags);
-
   DISALLOW_EVIL_CONSTRUCTORS(ChromeCanvas);
 };
 
diff --git a/chrome/views/label.cc b/chrome/views/label.cc
index ed8de0f..27f30d2 100644
--- a/chrome/views/label.cc
+++ b/chrome/views/label.cc
@@ -53,9 +53,8 @@ Label::~Label() {
 gfx::Size Label::GetPreferredSize() {
   gfx::Size prefsize;
   if (is_multi_line_) {
-    ChromeCanvas cc(0, 0, true);
     int w = width(), h = 0;
-    cc.SizeStringInt(text_, font_, &w, &h, ComputeMultiLineFlags());
+    ChromeCanvas::SizeStringInt(text_, font_, &w, &h, ComputeMultiLineFlags());
     prefsize.SetSize(w, h);
   } else {
     prefsize = GetTextSize();