Hide console log messages for imported WPT tests

content/shell/test_runner/web_frame_test_client.cc

 // 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 "content/shell/test_runner/web_frame_test_client.h"
 
 #include <memory>
 
 #include "base/logging.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "content/shell/test_runner/accessibility_controller.h"
 #include "content/shell/test_runner/event_sender.h"
 #include "content/shell/test_runner/mock_color_chooser.h"
 #include "content/shell/test_runner/mock_screen_orientation_client.h"
 #include "content/shell/test_runner/mock_web_user_media_client.h"
 #include "content/shell/test_runner/test_common.h"
 #include "content/shell/test_runner/test_interfaces.h"
 #include "content/shell/test_runner/test_plugin.h"
@@ skipping 601 matching lines @@
                                                 : mime_type.utf8().data());
     delegate_->PrintMessage("\n");
   }
 }
 
 void WebFrameTestClient::didAddMessageToConsole(
     const blink::WebConsoleMessage& message,
     const blink::WebString& source_name,
     unsigned source_line,
     const blink::WebString& stack_trace) {
   if (!test_runner()->ShouldDumpConsoleMessages())

[Rick Byers, 2017/02/24 22:02:27]

Wouldn't it be a lot simpler to just change the implementation of this function
so that it defaults to 'false' when test_runner()->is_web_platform_tests_mode_
is true?

If you did that, you'd avoid generating the console output in the first place
and so wouldn't need to filter it out later from python.  That would avoid
duplicating both the logic for what paths constitute a wpt directory, and what
prefixes are used for console output.  Also then there'd be no reason to make
this change for multi-line output (though maybe it's still worth doing for other
reasons - dunno).

[Dan Elphick, 2017/02/27 11:03:06]

Isn't this functionally equivalent to just calling setDumpConsoleMessages(false)
during the test for instance in testharnessreport.js when the test matches a wpt
directory? I suggested that in
https://bugs.chromium.org/p/chromium/issues/detail?id=679742#c3 and Quintin even
tested it, but there was concern that this would completely remove the console
messages so it was harder to debug a failing test.

If our aim was to avoid duplicating the wpt detection code, we could perhaps
change this method to instead of calling PrintMessage, just output via
fprintf(stderr, ...). That would eliminate the python change at least.

[Rick Byers, 2017/02/28 16:29:32]

Ah, I'm sorry - I was lacking the context on the discussion here.  I just read
over the whole bug history.

One option that doesn't seem like it was discussed is to add a new method to
BlinkTestResultPrinter like PrintMessage but for debugging output which
shouldn't be included in the comparison (or maybe an option to the existing
method / IPC).  It looks like it would be pretty easy to handle this new path in
BlinkTestResultPrinter (similar to BlinkTestResultPrinter::AddErrorMessage) to
output only to stderr.

Would that have the same effect as what you're achieving here without the need /
bittleness of outputting then stripping the output?

I _think_ that could be nearly as simple as what you're doing here, but perhaps
I'm missing something.  If it is really more complex for some reason then I'd
personally be ok with the output-then-filter approach being done here.  But
since it's potentially brittle I'd want a second opinion from someone with more
experience - perhaps add dpranke@?

     return;
   std::string level;
   switch (message.level) {
     case blink::WebConsoleMessage::LevelVerbose:
       level = "DEBUG";
       break;
     case blink::WebConsoleMessage::LevelInfo:
       level = "MESSAGE";
       break;
     case blink::WebConsoleMessage::LevelWarning:
       level = "WARNING";
       break;
     case blink::WebConsoleMessage::LevelError:
       level = "ERROR";
       break;
     default:
       level = "MESSAGE";
   }
-  delegate_->PrintMessage(std::string("CONSOLE ") + level + ": ");
+  std::string prefix(std::string("CONSOLE ") + level + ": ");
   if (source_line) {
-    delegate_->PrintMessage(base::StringPrintf("line %d: ", source_line));
+    prefix += base::StringPrintf("line %d: ", source_line);
   }
   if (!message.text.isEmpty()) {
     std::string new_message;
     new_message = message.text.utf8();
     size_t file_protocol = new_message.find("file://");
     if (file_protocol != std::string::npos) {
       new_message = new_message.substr(0, file_protocol) +
                     URLSuitableForTestResult(new_message.substr(file_protocol));
     }
-    delegate_->PrintMessage(new_message);
+    for (auto s : base::SplitStringPieceUsingSubstr(
+             new_message, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL)) {
+      delegate_->PrintMessage(prefix);
+      delegate_->PrintMessage(s.as_string());
+      delegate_->PrintMessage(std::string("\n"));
+    }
+  } else {
+    delegate_->PrintMessage(prefix + "\n");
   }
-  delegate_->PrintMessage(std::string("\n"));
 }
 
 blink::WebNavigationPolicy WebFrameTestClient::decidePolicyForNavigation(
     const blink::WebFrameClient::NavigationPolicyInfo& info) {
   // PlzNavigate
   // Navigation requests initiated by the renderer have checked navigation
   // policy when the navigation was sent to the browser. Some layout tests
   // expect that navigation policy is only checked once.
   if (delegate_->IsNavigationInitiatedByRenderer(info.urlRequest))
     return info.defaultPolicy;
@@ skipping 57 matching lines @@
 blink::WebEffectiveConnectionType
 WebFrameTestClient::getEffectiveConnectionType() {
   return test_runner()->effective_connection_type();
 }
 
 TestRunner* WebFrameTestClient::test_runner() {
   return web_view_test_proxy_base_->test_interfaces()->GetTestRunner();
 }
 
 }  // namespace test_runner