Add conditional evaluation to CRSBLOG

LOG doesn't even evaluate the streamed arguments if the conditions for
logging aren't met (verbosity, debug, etc.) Now CRSBLOG does the same,
so that CRSBLOG << Function() is a noop if there are no listening
chrome://safe-browsing tabs.

Bug: 874693
Change-Id: I6e1316b280b87264e65cb789151cff8596eb380a
Reviewed-on: https://chromium-review.googlesource.com/1187008
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586346}

components/BUILD.gn

@@ -239,6 +239,7 @@ test("components_unittests") {
       "//components/safe_browsing/common:unit_tests",
       "//components/safe_browsing/password_protection:password_protection_unittest",
       "//components/safe_browsing/triggers:unit_tests",
+      "//components/safe_browsing/web_ui:unit_tests",
       "//components/security_interstitials/content:unit_tests",
       "//components/security_state/content:unit_tests",
       "//components/services/heap_profiling:unit_tests",

components/safe_browsing/DEPS

@@ -5,6 +5,7 @@ include_rules = [
   "+components/sync/protocol",
   "+content/public/browser",
   "+content/public/common",
+  "+content/public/test",
   "+google_apis",
   "+mojo/public/cpp",
   "+net/base",

components/safe_browsing/web_ui/BUILD.gn

@@ -18,7 +18,6 @@ static_library("web_ui") {
     "//components/resources:components_scaled_resources_grit",
     "//components/safe_browsing:csd_proto",
     "//components/safe_browsing:features",
-    "//components/safe_browsing:webui_proto",
     "//components/safe_browsing/browser:referrer_chain_provider",
     "//components/safe_browsing/common:safe_browsing_prefs",
     "//components/safe_browsing/db:v4_local_database_manager",
@@ -29,6 +28,10 @@ static_library("web_ui") {
     "//net",
     "//url",
   ]
+
+  public_deps = [
+    "//components/safe_browsing:webui_proto",
+  ]
 }
 
 static_library("constants") {
@@ -37,3 +40,16 @@ static_library("constants") {
     "constants.h",
   ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+  sources = [
+    "safe_browsing_ui_unittest.cc",
+  ]
+
+  deps = [
+    ":web_ui",
+    "//content/test:test_support",
+    "//testing/gtest",
+  ]
+}

components/safe_browsing/web_ui/safe_browsing_ui.cc

@@ -31,6 +31,7 @@
 #include "components/strings/grit/components_strings.h"
 #include "components/user_prefs/user_prefs.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/web_contents.h"
 
 #if SAFE_BROWSING_DB_LOCAL
 #include "components/safe_browsing/db/v4_local_database_manager.h"
@@ -1245,6 +1246,10 @@ void SafeBrowsingUIHandler::RegisterMessages() {
                           base::Unretained(this)));
 }
 
+void SafeBrowsingUIHandler::SetWebUIForTesting(content::WebUI* web_ui) {
+  set_web_ui(web_ui);
+}
+
 CrSBLogMessage::CrSBLogMessage() {}
 
 CrSBLogMessage::~CrSBLogMessage() {

components/safe_browsing/web_ui/safe_browsing_ui.h

@@ -9,7 +9,6 @@
 #include "components/safe_browsing/proto/csd.pb.h"
 #include "components/safe_browsing/proto/webui.pb.h"
 #include "components/sync/protocol/user_event_specifics.pb.h"
-#include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/browser/web_ui_data_source.h"
 #include "content/public/browser/web_ui_message_handler.h"
@@ -76,6 +75,9 @@ class SafeBrowsingUIHandler : public content::WebUIMessageHandler {
   // Register callbacks for WebUI messages.
   void RegisterMessages() override;
 
+  // Sets the WebUI for testing
+  void SetWebUIForTesting(content::WebUI* web_ui);
+
  private:
   friend class WebUIInfoSingleton;
 
@@ -302,6 +304,9 @@ class WebUIInfoSingleton {
   DISALLOW_COPY_AND_ASSIGN(WebUIInfoSingleton);
 };
 
+// Used for streaming messages to the WebUIInfoSingleton. Collects streamed
+// messages, then sends them to the WebUIInfoSingleton when destroyed. Intended
+// to be used in CRSBLOG macro.
 class CrSBLogMessage {
  public:
   CrSBLogMessage();
@@ -313,7 +318,22 @@ class CrSBLogMessage {
   std::ostringstream stream_;
 };
 
-#define CRSBLOG CrSBLogMessage().stream()
+// Used to consume a stream so that we don't even evaluate the streamed data if
+// there are no chrome://safe-browsing tabs open.
+class CrSBLogVoidify {
+ public:
+  CrSBLogVoidify() = default;
+
+  // This has to be an operator with a precedence lower than <<,
+  // but higher than ?:
+  void operator&(std::ostream&) {}
+};
+
+#define CRSBLOG                                         \
+  (!::safe_browsing::WebUIInfoSingleton::HasListener()) \
+      ? static_cast<void>(0)                            \
+      : ::safe_browsing::CrSBLogVoidify() &             \
+            ::safe_browsing::CrSBLogMessage().stream()
 
 }  // namespace safe_browsing
 

components/safe_browsing/web_ui/safe_browsing_ui_unittest.cc

+// Copyright 2018 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 "components/safe_browsing/web_ui/safe_browsing_ui.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_web_ui.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace safe_browsing {
+
+class SafeBrowsingUITest : public testing::Test {
+ public:
+  SafeBrowsingUITest() {}
+
+  void SetUp() override {}
+
+  int SetMemberInt(int member_int) {
+    member_int_ = member_int;
+    return member_int_;
+  }
+
+  SafeBrowsingUIHandler* RegisterNewHandler() {
+    auto handler_unique =
+        std::make_unique<SafeBrowsingUIHandler>(&browser_context_);
+
+    SafeBrowsingUIHandler* handler = handler_unique.get();
+    handler->SetWebUIForTesting(&web_ui_);
+    WebUIInfoSingleton::GetInstance()->RegisterWebUIInstance(handler);
+
+    web_ui_.AddMessageHandler(std::move(handler_unique));
+    return handler;
+  }
+
+  void UnregisterHandler(SafeBrowsingUIHandler* handler) {
+    WebUIInfoSingleton::GetInstance()->UnregisterWebUIInstance(handler);
+  }
+
+ protected:
+  int member_int_;
+  content::TestWebUI web_ui_;
+  content::TestBrowserThreadBundle thread_bundle_;
+  content::TestBrowserContext browser_context_;
+};
+
+TEST_F(SafeBrowsingUITest, CRSBLOGDoesNotEvaluateWhenNoListeners) {
+  member_int_ = 0;
+
+  // Start with no listeners, so SetMemberInt() should not be evaluated.
+  CRSBLOG << SetMemberInt(1);
+  EXPECT_EQ(member_int_, 0);
+
+  // Register a listener, so SetMemberInt() will be evaluated.
+  SafeBrowsingUIHandler* handler = RegisterNewHandler();
+
+  CRSBLOG << SetMemberInt(1);
+  EXPECT_EQ(member_int_, 1);
+
+  UnregisterHandler(handler);
+}
+
+}  // namespace safe_browsing