Reduce the number of text reshaping in RenderText

ui/gfx/render_text_mac.cc

Index: ui/gfx/render_text_mac.cc
diff --git a/ui/gfx/render_text_mac.cc b/ui/gfx/render_text_mac.cc
index aa33fc019d26e165ed49205b0ab383540d0b4add..70df27ff3e16edd3fbd6154d0e25b072d9739a7f 100644
--- a/ui/gfx/render_text_mac.cc
+++ b/ui/gfx/render_text_mac.cc
@@ -17,7 +17,8 @@
 
 namespace gfx {
 
-RenderTextMac::RenderTextMac() : common_baseline_(0), runs_valid_(false) {
+RenderTextMac::RenderTextMac()
+    : common_baseline_(0), runs_valid_(false) {
 }
 
 RenderTextMac::~RenderTextMac() {
@@ -27,6 +28,14 @@ scoped_ptr<RenderText> RenderTextMac::CreateInstanceOfSameType() const {
   return scoped_ptr<RenderTextMac>(new RenderTextMac);
 }
 
+const base::string16& RenderTextMac::GetLayoutText() {
+  if (elide_behavior() == NO_ELIDE ||

[msw, 2015/02/13 05:53:37]

Is this check and early return really needed? Won't text_elided be false in
these cases anyway and cause layout_text() to be returned below anyway?

[oshima, 2015/02/13 21:03:11]

Nope. This was from attempt to do something similar on mac, but i gave up.

+      elide_behavior() == FADE_TAIL) {
+    return layout_text();
+  }
+  return text_elided() ? display_text() : layout_text();
+}
+
 Size RenderTextMac::GetStringSize() {
   EnsureLayout();
   return Size(std::ceil(string_size_.width()), string_size_.height());
@@ -87,12 +96,12 @@ std::vector<Rect> RenderTextMac::GetSubstringBounds(const Range& range) {
   return std::vector<Rect>();
 }
 
-size_t RenderTextMac::TextIndexToLayoutIndex(size_t index) const {
+size_t RenderTextMac::TextIndexToLayoutIndex(size_t index) {
   // TODO(asvitkine): Implement this. http://crbug.com/131618
   return index;
 }
 
-size_t RenderTextMac::LayoutIndexToTextIndex(size_t index) const {
+size_t RenderTextMac::LayoutIndexToTextIndex(size_t index) {
   // TODO(asvitkine): Implement this. http://crbug.com/131618
   return index;
 }
@@ -102,13 +111,23 @@ bool RenderTextMac::IsValidCursorIndex(size_t index) {
   return IsValidLogicalIndex(index);
 }
 
-void RenderTextMac::ResetLayout() {
+void RenderTextMac::OnLayoutTextAttributeChanged(bool text_changed) {
+  if (text_changed &&

[msw, 2015/02/13 05:53:37]

Can you explain the reasoning here? Why only conditionally UpdateDisplayText?

[oshima, 2015/02/13 21:03:11]

RenderText used to elide text when text changed, and this is to
keep the same behavior. I tried to change it but it's hard to do it
without having mac.

[msw, 2015/02/13 22:19:05]

I wonder if this needs to unconditionally update the display text... (eg. what
if the text didn't change but the elide_behavior changed from ELIDE_TAIL to
NO_ELIDE? Won't the display text be left elided?)

[oshima, 2015/02/13 23:24:11]

Ah, you're right. I have to call UpdateDisplayText(0) for the same reason RTHB
is doing
in that case. I think it should do the same as original version now.

+      elide_behavior() != NO_ELIDE &&
+      elide_behavior() != FADE_TAIL &&
+      !layout_text().empty()) {
+    UpdateDisplayText(GetContentWidth());

[msw, 2015/02/13 05:53:37]

Won't passing GetContentWidth here cause the text to never elide?

[oshima, 2015/02/13 21:03:11]

This is one of the issue I mentioned in the doc.
The very first GetContentWith() computes the width without eliding,
which gives the full width, and then this re-computes the layout text based on
that width and display rect.

As a matter of fact, this is causing an issue to mukai's label caching. 

[msw, 2015/02/13 21:49:23]

Acknowledged. (I misread this late last night...)

+  }
   line_.reset();
   attributes_.reset();
   runs_.clear();
   runs_valid_ = false;
 }
 
+void RenderTextMac::OnDisplayTextAttributeChanged() {
+  OnLayoutTextAttributeChanged(true);
+}
+
 void RenderTextMac::EnsureLayout() {
   if (line_.get())
     return;