Make ScrollPaintPropertyNode::toString() 26% more concise

third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp

Index: third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp b/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
index e3a09aa3a98d8733633586aa71c53378ce9af4d3..24afc52f450b7877f176d682243572737d5a6e8f 100644
--- a/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
+++ b/third_party/WebKit/Source/platform/graphics/paint/ScrollPaintPropertyNode.cpp
@@ -16,18 +16,31 @@ ScrollPaintPropertyNode* ScrollPaintPropertyNode::root() {
 }
 
 String ScrollPaintPropertyNode::toString() const {
-  std::string mainThreadScrollingReasonsAsText =
-      MainThreadScrollingReason::mainThreadScrollingReasonsAsText(
-          m_mainThreadScrollingReasons);
-  return String::format(
-      "parent=%p clip=%s bounds=%s "
-      "userScrollableHorizontal=%s"
-      " userScrollableVertical=%s mainThreadScrollingReasons=%s",
-      m_parent.get(), m_clip.toString().ascii().data(),
-      m_bounds.toString().ascii().data(),
-      m_userScrollableHorizontal ? "yes" : "no",
-      m_userScrollableVertical ? "yes" : "no",
-      mainThreadScrollingReasonsAsText.c_str());
+  StringBuilder text;
+  text.append("parent=");
+  text.append(String::format("%p", m_parent.get()));
+  text.append(" clip=");
+  text.append(m_clip.toString());
+  text.append(" bounds=");
+  text.append(m_bounds.toString());
+
+  text.append(" userScrollable=");
+  if (m_userScrollableHorizontal && m_userScrollableVertical)
+    text.append("both");
+  else if (!m_userScrollableHorizontal && !m_userScrollableHorizontal)

[brucedawson, 2017/01/31 20:21:28]

The experimental /analyze builder pointed out that this code is probably
incorrect. It said:

third_party\webkit\source\platform\graphics\paint\scrollpaintpropertynode.cpp(30)
: warning C6287: Redundant code:  the left and right sub-expressions are
identical.

And indeed, it looks like the check should have been:

if (!m_userScrollableHorizontal && !m_userScrollableVertical)

I've created CL https://codereview.chromium.org/2664253003 to address this.

+    text.append("none");
+  else
+    text.append(m_userScrollableHorizontal ? "horizontal" : "vertical");
+
+  text.append(" mainThreadReasons=");
+  if (m_mainThreadScrollingReasons) {
+    text.append(MainThreadScrollingReason::mainThreadScrollingReasonsAsText(
+                    m_mainThreadScrollingReasons)
+                    .c_str());
+  } else {
+    text.append("none");
+  }
+  return text.toString();
 }
 
 #if DCHECK_IS_ON()