Fix incorrect console message for subframe navigation filtering

1. Currently there is a bug: when subframe navigation is blocked, the
console message is like: |<URL> on this site ...|; while the expected
message should be |Chrome blocked resource <URL> on this site ...|. The
reason is that in the code the prefix is put in the constructor of
ostringstream but that call doesn't advance the write pointer.

There was a test for it but the test also did it in the wrong way.

2. Fixed another potential issue:
Test SubresourceFilterBrowserTest.SubFrameActivation is referring to
kDisallowSubframeConsoleMessagePrefix/Suffix but the generated string actually
comes from GetErrorStringForDisallowedLoad() in
blink/renderer/core/loader/subresource_filter.cc. Right now the two
strings are the same thus the test is passing now, but this should be
fixed.


Bug: N/A
Change-Id: I28548d809ecaae019321bf97551d2493f2af7703
Reviewed-on: https://chromium-review.googlesource.com/1237173Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593544}

chrome/browser/subresource_filter/subresource_filter_browsertest.cc

@@ -135,6 +135,13 @@ GURL GetURLWithFragment(const GURL& url, base::StringPiece fragment) {
   return url.ReplaceComponents(replacements);
 }
 
+// This string comes from GetErrorStringForDisallowedLoad() in
+// blink/renderer/core/loader/subresource_filter.cc
+constexpr const char kBlinkDisallowSubframeConsoleMessageFormat[] =
+    "Chrome blocked resource %s on this site because this site tends to show "
+    "ads that interrupt, distract, or prevent user control. Learn more at "
+    "https://www.chromestatus.com/feature/5738264052891648";
+
 }  // namespace
 
 // Tests -----------------------------------------------------------------------
@@ -280,10 +287,10 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, SubFrameActivation) {
-  std::ostringstream message_filter;
-  message_filter << kDisallowSubframeConsoleMessagePrefix << "*";
+  std::string message_filter =
+      base::StringPrintf(kBlinkDisallowSubframeConsoleMessageFormat, "*");
   content::ConsoleObserverDelegate console_observer(web_contents(),
-                                                    message_filter.str());
+                                                    message_filter);
   web_contents()->SetDelegate(&console_observer);
 
   GURL url(GetTestUrl(kTestFrameSetPath));
@@ -302,17 +309,18 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest, SubFrameActivation) {
                            SubresourceFilterAction::kUIShown, 1);
 
   // Console message for subframe blocking should be displayed.
-  std::ostringstream result;
-  result << kDisallowSubframeConsoleMessagePrefix << "*included_script.js*";
-  EXPECT_TRUE(base::MatchPattern(console_observer.message(), result.str()));
+  EXPECT_TRUE(base::MatchPattern(
+      console_observer.message(),
+      base::StringPrintf(kBlinkDisallowSubframeConsoleMessageFormat,
+                         "*included_script.js")));
 }
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
                        ActivationDisabled_NoConsoleMessage) {
-  std::ostringstream message_filter;
-  message_filter << kDisallowSubframeConsoleMessageSuffix << "*";
+  std::string message_filter =
+      base::StringPrintf(kBlinkDisallowSubframeConsoleMessageFormat, "*");
   content::ConsoleObserverDelegate console_observer(web_contents(),
-                                                    message_filter.str());
+                                                    message_filter);
   web_contents()->SetDelegate(&console_observer);
 
   Configuration config(
@@ -334,10 +342,10 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
                        ActivationDryRun_NoConsoleMessage) {
-  std::ostringstream message_filter;
-  message_filter << kDisallowSubframeConsoleMessageSuffix << "*";
+  std::string message_filter =
+      base::StringPrintf(kBlinkDisallowSubframeConsoleMessageFormat, "*");
   content::ConsoleObserverDelegate console_observer(web_contents(),
-                                                    message_filter.str());
+                                                    message_filter);
   web_contents()->SetDelegate(&console_observer);
 
   Configuration config(

components/subresource_filter/content/browser/subframe_navigation_filtering_throttle.cc

@@ -93,14 +93,14 @@ void SubframeNavigationFilteringThrottle::OnCalculatedLoadPolicy(
 
   if (policy == LoadPolicy::DISALLOW) {
     if (parent_frame_filter_->activation_state().enable_logging) {
-      std::ostringstream oss(kDisallowSubframeConsoleMessagePrefix);
-      oss << navigation_handle()->GetURL();
-      oss << kDisallowSubframeConsoleMessageSuffix;
+      std::string console_message = base::StringPrintf(
+          kDisallowSubframeConsoleMessageFormat,
+          navigation_handle()->GetURL().possibly_invalid_spec().c_str());
       navigation_handle()
           ->GetWebContents()
           ->GetMainFrame()
           ->AddMessageToConsole(content::CONSOLE_MESSAGE_LEVEL_ERROR,
-                                oss.str());
+                                console_message);
     }
 
     parent_frame_filter_->ReportDisallowedLoad();

components/subresource_filter/content/browser/subframe_navigation_filtering_throttle_unittest.cc

@@ -158,10 +158,8 @@ class SubframeNavigationFilteringThrottleTest
   }
 
   std::string GetFilterConsoleMessage(const GURL& filtered_url) {
-    std::ostringstream oss(kDisallowSubframeConsoleMessagePrefix);
-    oss << filtered_url;
-    oss << kDisallowSubframeConsoleMessageSuffix;
-    return oss.str();
+    return base::StringPrintf(kDisallowSubframeConsoleMessageFormat,
+                              filtered_url.possibly_invalid_spec().c_str());
   }
 
  private:

components/subresource_filter/core/browser/subresource_filter_constants.h

@@ -62,12 +62,9 @@ constexpr char kActivationWarningConsoleMessage[] =
     "https://www.chromestatus.com/feature/5738264052891648";
 
 // Console message to be displayed on disallowing subframe.
-constexpr char kDisallowSubframeConsoleMessagePrefix[] =
-    "Chrome blocked resource ";
-
-constexpr char kDisallowSubframeConsoleMessageSuffix[] =
-    " on this site because this site tends to show ads that interrupt, "
-    "distract, or prevent user control. Learn more at "
+constexpr char kDisallowSubframeConsoleMessageFormat[] =
+    "Chrome blocked resource %s on this site because this site tends to show "
+    "ads that interrupt, distract, or prevent user control. Learn more at "
     "https://www.chromestatus.com/feature/5738264052891648";
 
 constexpr char kLearnMoreLink[] =