Change error reporting of remote event logging in incognito mode

The feature of WebRTC event logging to a remote server, is now
intended to be gated behind a Chrome policy. As such, it would make
more sense to treat incognito mode as any other profile, which
does not have the policy enabled.

Bug: 775415
Change-Id: I14c0b5e14578fc8a5ae27d8921583728e8850282
Reviewed-on: https://chromium-review.googlesource.com/1105770
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569211}

chrome/browser/extensions/api/webrtc_logging_private/webrtc_logging_private_apitest.cc

@@ -88,8 +88,11 @@ class WebrtcLoggingPrivateApiTest : public extensions::ExtensionApiTest {
     return function;
   }
 
+  // Overriding can use incognito session instead, etc.
+  virtual Browser* GetBrowser() { return browser(); }
+
   content::WebContents* web_contents() {
-    return browser()->tab_strip_model()->GetActiveWebContents();
+    return GetBrowser()->tab_strip_model()->GetActiveWebContents();
   }
 
   void AppendTabIdAndUrl(base::ListValue* parameters) {
@@ -109,7 +112,7 @@ class WebrtcLoggingPrivateApiTest : public extensions::ExtensionApiTest {
   bool RunFunction(UIThreadExtensionFunction* function,
                    const base::ListValue& parameters) {
     std::unique_ptr<base::Value> result(utils::RunFunctionAndReturnSingleResult(
-        function, ParamsToString(parameters), browser()));
+        function, ParamsToString(parameters), GetBrowser()));
     return (result != nullptr);
   }
 
@@ -143,7 +146,7 @@ class WebrtcLoggingPrivateApiTest : public extensions::ExtensionApiTest {
     DCHECK(!expected_error.empty());
     scoped_refptr<Function> function(CreateFunction<Function>());
     const std::string error_message = utils::RunFunctionAndReturnError(
-        function.get(), ParamsToString(parameters), browser());
+        function.get(), ParamsToString(parameters), GetBrowser());
     EXPECT_EQ(error_message, expected_error);
   }
 
@@ -352,6 +355,20 @@ class WebrtcLoggingPrivateApiTestDisabledRemoteLogging
   }
 };
 
+class WebrtcLoggingPrivateApiTestInIncognitoMode
+    : public WebrtcLoggingPrivateApiTest {
+ protected:
+  Browser* GetBrowser() override {
+    if (!browser_) {
+      browser_ = CreateIncognitoBrowser();
+    }
+    return browser_;
+  }
+
+ private:
+  Browser* browser_{nullptr};  // Does not own the object.
+};
+
 // Helper class to temporarily tell the uploader to save the multipart buffer to
 // a test string instead of uploading.
 class ScopedOverrideUploadBuffer {
@@ -534,7 +551,7 @@ IN_PROC_BROWSER_TEST_F(WebrtcLoggingPrivateApiTest, TestStoreWithoutLog) {
   scoped_refptr<WebrtcLoggingPrivateStoreFunction> store(
       CreateFunction<WebrtcLoggingPrivateStoreFunction>());
   const std::string error = utils::RunFunctionAndReturnError(
-      store.get(), ParamsToString(parameters), browser());
+      store.get(), ParamsToString(parameters), GetBrowser());
   ASSERT_FALSE(error.empty());
 }
 
@@ -735,3 +752,15 @@ IN_PROC_BROWSER_TEST_F(WebrtcLoggingPrivateApiTestDisabledRemoteLogging,
   StartEventLogging(peer_connection_id, max_size_bytes, metadata,
                     expect_success, error_message);
 }
+
+IN_PROC_BROWSER_TEST_F(WebrtcLoggingPrivateApiTestInIncognitoMode,
+                       StartEventLoggingFails) {
+  const std::string peer_connection_id = "id";
+  SetUpPeerConnection(peer_connection_id);
+  const int max_size_bytes = kMaxRemoteLogFileSizeBytes;
+  const std::string metadata = "metadata";
+  constexpr bool expect_success = false;
+  const std::string error_message = kStartRemoteLoggingFailureFeatureDisabled;
+  StartEventLogging(peer_connection_id, max_size_bytes, metadata,
+                    expect_success, error_message);
+}

chrome/browser/media/webrtc/webrtc_event_log_manager.cc

@@ -299,17 +299,21 @@ void WebRtcEventLogManager::StartRemoteLogging(
   }
 
   const BrowserContext* browser_context = GetBrowserContext(render_process_id);
-  if (!browser_context || browser_context->IsOffTheRecord()) {
-    // RPH died before processing of this notification, or is incognito.
-    // In the former case, there's no one to report to anyway.
-    // In the latter case, we don't want to expose incognito state to the
-    // JS application, so we give an error message that must be shared with
-    // other common events.
+  if (!browser_context) {
+    // RPH died before processing of this notification.
     MaybeReply(std::move(reply), false,
                std::string(kStartRemoteLoggingFailureGeneric));
     return;
   }
 
+  if (browser_context->IsOffTheRecord()) {
+    // Feature disable in incognito. Since the feature can be disabled for
+    // non-incognito sessions, this should not expose incognito mode.
+    MaybeReply(std::move(reply), false,
+               std::string(kStartRemoteLoggingFailureFeatureDisabled));
+    return;
+  }
+
   const auto browser_context_id = GetBrowserContextId(browser_context);
   DCHECK_NE(browser_context_id, kNullBrowserContextId);
 

chrome/browser/media/webrtc/webrtc_event_log_manager_remote.cc

@@ -276,9 +276,8 @@ bool WebRtcRemoteEventLogManager::StartRemoteLogging(
 
   if (!AdditionalActiveLogAllowed(key.browser_context_id)) {
     // Intentionally use a generic error, so as to not leak information such
-    // as this being an incognito session (rejected elsewhere with the same
-    // error), or there being too many other peer connections on other tabs
-    // that might also be logging.
+    // as there being too many other peer connections on other tabs that might
+    // also be logging.
     *error_message = kStartRemoteLoggingFailureGeneric;
     return false;
   }