Single PrintViewHierarchy implementation.

Why did we have 2 impls and 3 sets of headers? Why did it use streams?

Bug: none
Low-Coverage-Reason: LARGE_SCALE_REFACTOR collapsing existing impls
Change-Id: Ie57200239469382feee663737d956ba438f640b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6111809
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Owners-Override: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Allen Bauer <kylixrd@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1398768}

ash/debug.cc

@@ -21,6 +21,7 @@
 #include "ui/compositor/debug_utils.h"
 #include "ui/compositor/layer.h"
 #include "ui/views/debug_utils.h"
+#include "ui/views/view_utils.h"
 #include "ui/views/widget/widget.h"
 
 namespace ash {
@@ -64,7 +65,8 @@ void PrintViewHierarchy(std::ostringstream* out) {
 
   *out << "Host widget:\n";
   views::PrintWidgetInformation(*widget, /*detailed*/ true, out);
-  views::PrintViewHierarchy(widget->GetRootView(), out);
+  *out << "View hierarchy:\n"
+       << views::PrintViewHierarchy(widget->GetRootView());
 }
 
 std::vector<std::string> PrintWindowHierarchy(std::ostringstream* out,

chrome/browser/ui/views/browser_commands_views.cc

@@ -64,12 +64,12 @@ void ExecuteUIDebugCommand(int id, const Browser* browser) {
       break;
     }
     case IDC_DEBUG_PRINT_VIEW_TREE:
-      if (views::View* view = GetActiveWindowRootView(browser))
-        PrintViewHierarchy(view);
-      break;
     case IDC_DEBUG_PRINT_VIEW_TREE_DETAILS:
-      if (views::View* view = GetActiveWindowRootView(browser))
-        PrintViewHierarchy(view, /* verbose= */ true);
+      if (views::View* view = GetActiveWindowRootView(browser)) {
+        LOG(ERROR) << '\n'
+                   << PrintViewHierarchy(
+                          view, id == IDC_DEBUG_PRINT_VIEW_TREE_DETAILS);
+      }
       break;
     default:
       NOTREACHED() << "Unimplemented UI Debug command: " << id;

tools/win/chromeexts/commands/view_command.cc

@@ -249,10 +249,9 @@ HRESULT ViewCommand::Execute() {
 
     if (command_line().HasSwitch("r")) {
       DebugOutputBuffer buffer(GetDebugClientAs<IDebugControl>().Get());
-      std::ostream out(&buffer);
       VirtualViewDebugWrapper root(view_block,
                                    GetDebugClientAs<IDebugClient>().Get());
-      PrintViewHierarchy(&out, &root);
+      std::ostream(&buffer) << PrintViewHierarchy(&root);
     } else {
       for (auto val : children_ptrs) {
         Printf("%x ", val);

ui/views/debug/debugger_utils.cc

@@ -6,7 +6,10 @@
 
 #include <inttypes.h>
 
+#include <string>
+
 #include "base/logging.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 
@@ -82,50 +85,50 @@ std::string GetPaddedLine(int current_depth, bool attribute_line = false) {
   return std::string(padding, ' ');
 }
 
-void PrintViewHierarchyImpl(std::ostream* out,
-                            ViewDebugWrapper* view,
-                            int current_depth,
-                            bool verbose,
-                            int target_depth,
-                            size_t column_limit) {
-  std::string line = GetPaddedLine(current_depth);
-
-  line += "<" + view->GetViewClassName();
+std::string PrintViewHierarchyImpl(ViewDebugWrapper* view,
+                                   bool verbose,
+                                   int current_depth) {
+  std::string output;
+  std::string line = base::StrCat(
+      {GetPaddedLine(current_depth), "<", view->GetViewClassName()});
 
   for (const std::string& attribute_string :
        GetAttributeStrings(view, verbose)) {
-    if (line.size() + attribute_string.size() + 1 > column_limit) {
+    static constexpr size_t kColumnLimit = 240;
+    if (line.size() + attribute_string.size() + 1 > kColumnLimit) {
       // If adding the attribute string would cause the line to exceed the
       // column limit, send the line to `out` and start a new line. The new line
       // should fit at least one attribute string even if it means exceeding the
       // column limit.
-      *out << line << "\n";
+      output += line;
+      output += "\n";
       line = GetPaddedLine(current_depth, true) + attribute_string;
     } else {
       // Keep attribute strings on the existing line if it fits within the
       // column limit.
-      line += " " + attribute_string;
+      line += " ";
+      line += attribute_string;
     }
   }
 
   // Print children only if they exist and we are not yet at our target tree
   // depth.
-  if (!view->GetChildren().empty() &&
-      (target_depth == -1 || current_depth < target_depth)) {
-    *out << line << ">\n";
+  output += line;
+  if (!view->GetChildren().empty()) {
+    output += ">\n";
 
     for (ViewDebugWrapper* child : view->GetChildren()) {
-      PrintViewHierarchyImpl(out, child, current_depth + 1, verbose,
-                             target_depth, column_limit);
+      output += PrintViewHierarchyImpl(child, verbose, current_depth + 1);
     }
 
-    line = GetPaddedLine(current_depth);
-    *out << line << "</" << view->GetViewClassName() << ">\n";
+    output += base::StrCat(
+        {GetPaddedLine(current_depth), "</", view->GetViewClassName(), ">\n"});
   } else {
     // If no children are to be printed use a self closing tag to terminate the
     // View element.
-    *out << line << " />\n";
+    output += " />\n";
   }
+  return output;
 }
 
 }  // namespace
@@ -134,12 +137,8 @@ std::optional<intptr_t> ViewDebugWrapper::GetAddress() {
   return std::nullopt;
 }
 
-void PrintViewHierarchy(std::ostream* out,
-                        ViewDebugWrapper* view,
-                        bool verbose,
-                        int depth,
-                        size_t column_limit) {
-  PrintViewHierarchyImpl(out, view, 0, verbose, depth, column_limit);
+std::string PrintViewHierarchy(ViewDebugWrapper* view, bool verbose) {
+  return PrintViewHierarchyImpl(view, verbose, 0);
 }
 
 }  // namespace views::debug

ui/views/debug/debugger_utils.h

@@ -5,8 +5,9 @@
 #ifndef UI_VIEWS_DEBUG_DEBUGGER_UTILS_H_
 #define UI_VIEWS_DEBUG_DEBUGGER_UTILS_H_
 
+#include <stdint.h>
+
 #include <optional>
-#include <ostream>
 #include <string>
 #include <tuple>
 #include <vector>
@@ -16,8 +17,8 @@
 namespace views::debug {
 
 // This class acts as a "view" over the View class. This has been done to allow
-// debugger extensions to remnain resillient to structure and version changes in
-// the code base.
+// debugger extensions to remain resilient to structure and version changes in
+// the codebase.
 // TODO(tluk): Replace use of //ui/views/debug_utils.h with this.
 class ViewDebugWrapper {
  public:
@@ -41,11 +42,7 @@ class ViewDebugWrapper {
   virtual std::optional<intptr_t> GetAddress();
 };
 
-void PrintViewHierarchy(std::ostream* out,
-                        ViewDebugWrapper* view,
-                        bool verbose = false,
-                        int depth = -1,
-                        size_t column_limit = 240);
+std::string PrintViewHierarchy(ViewDebugWrapper* view, bool verbose = false);
 
 }  // namespace views::debug
 

ui/views/debug_utils.cc

@@ -20,24 +20,6 @@
 
 namespace views {
 namespace {
-void PrintViewHierarchyImp(const View* view,
-                           size_t indent,
-                           std::ostringstream* out) {
-  *out << std::string(indent, ' ');
-  *out << view->GetClassName();
-  *out << ' ';
-  *out << view->GetID();
-  *out << ' ';
-  *out << view->x() << "," << view->y() << ",";
-  *out << view->bounds().right() << "," << view->bounds().bottom();
-  *out << ' ';
-  *out << view;
-  *out << '\n';
-
-  for (const View* child : view->children())
-    PrintViewHierarchyImp(child, indent + 2, out);
-}
-
 void PrintFocusHierarchyImp(const View* view,
                             size_t indent,
                             std::ostringstream* out) {
@@ -153,18 +135,6 @@ void PrintWidgetInformation(const Widget& widget,
   *out << '\n';
 }
 
-void PrintViewHierarchy(const View* view) {
-  std::ostringstream out;
-  PrintViewHierarchy(view, &out);
-  // Error so users in the field can generate and upload logs.
-  LOG(ERROR) << out.str();
-}
-
-void PrintViewHierarchy(const View* view, std::ostringstream* out) {
-  *out << "View hierarchy:\n";
-  PrintViewHierarchyImp(view, 0, out);
-}
-
 void PrintFocusHierarchy(const View* view) {
   std::ostringstream out;
   out << "Focus hierarchy:\n";

ui/views/debug_utils.h

@@ -15,12 +15,6 @@ namespace views {
 class View;
 class Widget;
 
-// Log the view hierarchy.
-VIEWS_EXPORT void PrintViewHierarchy(const View* view);
-
-// Print the view hierarchy to |out|.
-VIEWS_EXPORT void PrintViewHierarchy(const View* view, std::ostringstream* out);
-
 // Log the focus traversal hierarchy.
 VIEWS_EXPORT void PrintFocusHierarchy(const View* view);
 

ui/views/examples/button_example.cc

@@ -289,7 +289,7 @@ void ButtonExample::LabelButtonPressed(LabelButton* label_button,
     label_button->SetIsDefault(!label_button->GetIsDefault());
   }
   example_view()->GetLayoutManager()->Layout(example_view());
-  PrintViewHierarchy(example_view());
+  LOG(ERROR) << '\n' << PrintViewHierarchy(example_view());
 }
 
 void ButtonExample::ImageButtonPressed() {

ui/views/view_utils.cc

@@ -81,11 +81,9 @@ void ViewDebugWrapperImpl::ForAllProperties(PropCallback callback) {
   }
 }
 
-void PrintViewHierarchy(View* view, bool verbose, int depth) {
+std::string PrintViewHierarchy(View* view, bool verbose) {
   ViewDebugWrapperImpl debug_view(view);
-  std::ostringstream out;
-  debug::PrintViewHierarchy(&out, &debug_view, verbose, depth);
-  LOG(ERROR) << '\n' << out.str();
+  return debug::PrintViewHierarchy(&debug_view, verbose);
 }
 
 std::string GetViewDebugInfo(View* view) {

ui/views/view_utils.h

@@ -61,9 +61,7 @@ const V* AsViewClass(const View* view) {
   return IsViewClass<V>(view) ? static_cast<const V*>(view) : nullptr;
 }
 
-VIEWS_EXPORT void PrintViewHierarchy(View* view,
-                                     bool verbose = false,
-                                     int depth = -1);
+VIEWS_EXPORT std::string PrintViewHierarchy(View* view, bool verbose = false);
 
 VIEWS_EXPORT std::string GetViewDebugInfo(View* view);