Make ScrollPaintPropertyNode::toString() 26% more concise

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

 // Copyright 2016 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 "platform/graphics/paint/ScrollPaintPropertyNode.h"
 
 #include "platform/graphics/paint/PropertyTreeState.h"
 
 namespace blink {
 
 ScrollPaintPropertyNode* ScrollPaintPropertyNode::root() {
   DEFINE_STATIC_REF(ScrollPaintPropertyNode, root,
                     (ScrollPaintPropertyNode::create(
                         nullptr, IntSize(), IntSize(), false, false, 0)));
   return 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()
 
 String ScrollPaintPropertyNode::toTreeString() const {
   return blink::PropertyTreeStatePrinter<blink::ScrollPaintPropertyNode>()
       .pathAsString(this);
 }
 
 #endif
 
 }  // namespace blink