WebRtcEventLogManager: Remove default values of virtual functions

Bug: 775415
Change-Id: I69c7fcae44cd2d330c175da5ad588d861fd17e95
Reviewed-on: https://chromium-review.googlesource.com/1116793
Commit-Queue: Elad Alon <eladalon@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571181}

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

@@ -336,7 +336,8 @@ class WebrtcLoggingPrivateApiTest : public extensions::ExtensionApiTest {
     const int render_process_id = rph->GetID();
     const int lid = 0;
 
-    manager->PeerConnectionAdded(render_process_id, lid, peer_connection_id);
+    manager->PeerConnectionAdded(render_process_id, lid, peer_connection_id,
+                                 base::OnceCallback<void(bool)>());
   }
 
   base::test::ScopedFeatureList scoped_feature_list_;

chrome/browser/media/webrtc/webrtc_event_log_manager.cc

@@ -64,18 +64,6 @@ inline void MaybeReply(const base::Location& location,
   }
 }
 
-// No style-guide-permited way of forcing const-ref inference at the moment.
-inline void MaybeReply(const base::Location& location,
-                       base::OnceCallback<void(bool, const std::string&)> reply,
-                       bool bool_val,
-                       const std::string& str_val) {
-  if (reply) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, location,
-        base::BindOnce(std::move(reply), bool_val, str_val));
-  }
-}
-
 }  // namespace
 
 const size_t kWebRtcEventLogManagerUnlimitedFileSize = 0;
@@ -312,6 +300,7 @@ void WebRtcEventLogManager::StartRemoteLogging(
     size_t max_file_size_bytes,
     base::OnceCallback<void(bool, const std::string&)> reply) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(reply);
 
   const BrowserContext* browser_context = GetBrowserContext(render_process_id);
   const char* error = nullptr;
@@ -328,7 +317,9 @@ void WebRtcEventLogManager::StartRemoteLogging(
   }
 
   if (error) {
-    MaybeReply(FROM_HERE, std::move(reply), false, std::string(error));
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::BindOnce(std::move(reply), false, std::string(error)));
     return;
   }
 
@@ -624,8 +615,9 @@ void WebRtcEventLogManager::StartRemoteLoggingInternal(
       browser_context_dir, max_file_size_bytes, &error_message);
   DCHECK_EQ(result, error_message.empty());  // Error set iff has failed.
 
-  MaybeReply(FROM_HERE,
-             base::BindOnce(std::move(reply), result, error_message));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(std::move(reply), result, error_message));
 }
 
 void WebRtcEventLogManager::ClearCacheForBrowserContextInternal(

chrome/browser/media/webrtc/webrtc_event_log_manager.h

@@ -70,52 +70,43 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
 
   ~WebRtcEventLogManager() override;
 
-  void EnableForBrowserContext(
-      const content::BrowserContext* browser_context,
-      base::OnceClosure reply = base::OnceClosure()) override;
+  void EnableForBrowserContext(const content::BrowserContext* browser_context,
+                               base::OnceClosure reply) override;
 
-  void DisableForBrowserContext(
-      const content::BrowserContext* browser_context,
-      base::OnceClosure reply = base::OnceClosure()) override;
+  void DisableForBrowserContext(const content::BrowserContext* browser_context,
+                                base::OnceClosure reply) override;
 
   void PeerConnectionAdded(int render_process_id,
                            int lid,  // Renderer-local PeerConnection ID.
                            const std::string& peer_connection_id,
-                           base::OnceCallback<void(bool)> reply =
-                               base::OnceCallback<void(bool)>()) override;
+                           base::OnceCallback<void(bool)> reply) override;
 
   void PeerConnectionRemoved(int render_process_id,
                              int lid,  // Renderer-local PeerConnection ID.
-                             base::OnceCallback<void(bool)> reply =
-                                 base::OnceCallback<void(bool)>()) override;
+                             base::OnceCallback<void(bool)> reply) override;
 
   // From the logger's perspective, we treat stopping a peer connection the
   // same as we do its removal. Should a stopped peer connection be later
   // removed, the removal callback will assume the value |false|.
   void PeerConnectionStopped(int render_process_id,
                              int lid,  // Renderer-local PeerConnection ID.
-                             base::OnceCallback<void(bool)> reply =
-                                 base::OnceCallback<void(bool)>()) override;
+                             base::OnceCallback<void(bool)> reply) override;
 
   // The file's actual path is derived from |base_path| by adding a timestamp,
   // the render process ID and the PeerConnection's local ID.
   void EnableLocalLogging(const base::FilePath& base_path,
-                          base::OnceCallback<void(bool)> reply =
-                              base::OnceCallback<void(bool)>()) override;
-  void EnableLocalLogging(
-      const base::FilePath& base_path,
-      size_t max_file_size_bytes = kDefaultMaxLocalLogFileSizeBytes,
-      base::OnceCallback<void(bool)> reply = base::OnceCallback<void(bool)>());
+                          base::OnceCallback<void(bool)> reply) override;
+  void EnableLocalLogging(const base::FilePath& base_path,
+                          size_t max_file_size_bytes,
+                          base::OnceCallback<void(bool)> reply);
 
-  void DisableLocalLogging(base::OnceCallback<void(bool)> reply =
-                               base::OnceCallback<void(bool)>()) override;
+  void DisableLocalLogging(base::OnceCallback<void(bool)> reply) override;
 
   void OnWebRtcEventLogWrite(
       int render_process_id,
       int lid,  // Renderer-local PeerConnection ID.
       const std::string& message,
-      base::OnceCallback<void(std::pair<bool, bool>)> reply =
-          base::OnceCallback<void(std::pair<bool, bool>)>()) override;
+      base::OnceCallback<void(std::pair<bool, bool>)> reply) override;
 
   // Start logging a peer connection's WebRTC events to a file, which will
   // later be uploaded to a remote server. If a reply is provided, it will be
@@ -126,8 +117,7 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
       int render_process_id,
       const std::string& peer_connection_id,
       size_t max_file_size_bytes,
-      base::OnceCallback<void(bool, const std::string&)> reply =
-          base::OnceCallback<void(bool, const std::string&)>());
+      base::OnceCallback<void(bool, const std::string&)> reply);
 
   // Clear WebRTC event logs associated with a given browser context, in a given
   // time range (|delete_begin| inclusive, |delete_end| exclusive), then
@@ -146,7 +136,7 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
   // If a reply callback is given, it will be posted back to BrowserThread::UI
   // after the observer has been set.
   void SetLocalLogsObserver(WebRtcLocalEventLogsObserver* observer,
-                            base::OnceClosure reply = base::OnceClosure());
+                            base::OnceClosure reply);
 
   // Set (or unset) an observer that will be informed whenever a remote log file
   // is started/stopped. Note that this refers to writing these files to disk,
@@ -157,7 +147,7 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
   // If a reply callback is given, it will be posted back to BrowserThread::UI
   // after the observer has been set.
   void SetRemoteLogsObserver(WebRtcRemoteEventLogsObserver* observer,
-                             base::OnceClosure reply = base::OnceClosure());
+                             base::OnceClosure reply);
 
  private:
   friend class SigninManagerAndroidTest;       // Calls *ForTesting() methods.
@@ -250,8 +240,7 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
   // Injects a fake clock, to be used by tests. For example, this could be
   // used to inject a frozen clock, thereby allowing unit tests to know what a
   // local log's filename would end up being.
-  void SetClockForTesting(base::Clock* clock,
-                          base::OnceClosure reply = base::OnceClosure());
+  void SetClockForTesting(base::Clock* clock, base::OnceClosure reply);
 
   // Injects a PeerConnectionTrackerProxy for testing. The normal tracker proxy
   // is used to communicate back to WebRTC whether event logging is desired for
@@ -259,12 +248,12 @@ class WebRtcEventLogManager final : public content::RenderProcessHostObserver,
   // intercepted by a unit test.
   void SetPeerConnectionTrackerProxyForTesting(
       std::unique_ptr<PeerConnectionTrackerProxy> pc_tracker_proxy,
-      base::OnceClosure reply = base::OnceClosure());
+      base::OnceClosure reply);
 
   // Injects a fake uploader, to be used by unit tests.
   void SetWebRtcEventLogUploaderFactoryForTesting(
       std::unique_ptr<WebRtcEventLogUploader::Factory> uploader_factory,
-      base::OnceClosure reply = base::OnceClosure());
+      base::OnceClosure reply);
 
   // This allows unit tests that do not wish to change the task runner to still
   // check when certain operations are finished.

content/browser/browser_context.cc

@@ -570,7 +570,7 @@ void BrowserContext::Initialize(
   if (!browser_context->IsOffTheRecord()) {
     WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
     if (logger) {
-      logger->EnableForBrowserContext(browser_context);
+      logger->EnableForBrowserContext(browser_context, base::OnceClosure());
     }
   }
 }
@@ -627,7 +627,7 @@ BrowserContext::~BrowserContext() {
 
   WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
   if (logger) {
-    logger->DisableForBrowserContext(this);
+    logger->DisableForBrowserContext(this, base::OnceClosure());
   }
 
   RemoveBrowserContextFromUserIdMap(this);

content/browser/renderer_host/media/peer_connection_tracker_host.cc

@@ -62,16 +62,19 @@ void PeerConnectionTrackerHost::OnChannelClosing() {
 void PeerConnectionTrackerHost::OnAddPeerConnection(
     const PeerConnectionInfo& info) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   WebRTCInternals* webrtc_internals = WebRTCInternals::GetInstance();
   if (webrtc_internals) {
     webrtc_internals->OnAddPeerConnection(
         render_process_id_, peer_pid(), info.lid, info.url,
         info.rtc_configuration, info.constraints);
   }
+
   WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
   if (logger) {
     logger->PeerConnectionAdded(render_process_id_, info.lid,
-                                info.peer_connection_id);
+                                info.peer_connection_id,
+                                base::OnceCallback<void(bool)>());
   }
 }
 
@@ -89,7 +92,8 @@ void PeerConnectionTrackerHost::RemovePeerConnection(int lid) {
   }
   WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
   if (logger) {
-    logger->PeerConnectionRemoved(render_process_id_, lid);
+    logger->PeerConnectionRemoved(render_process_id_, lid,
+                                  base::OnceCallback<void(bool)>());
   }
 }
 
@@ -107,7 +111,8 @@ void PeerConnectionTrackerHost::UpdatePeerConnection(int lid,
   if (type == "stop") {
     WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
     if (logger) {
-      logger->PeerConnectionStopped(render_process_id_, lid);
+      logger->PeerConnectionStopped(render_process_id_, lid,
+                                    base::OnceCallback<void(bool)>());
     }
   }
 
@@ -157,7 +162,9 @@ void PeerConnectionTrackerHost::WebRtcEventLogWrite(int lid,
   }
   WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
   if (logger) {
-    logger->OnWebRtcEventLogWrite(render_process_id_, lid, output);
+    logger->OnWebRtcEventLogWrite(
+        render_process_id_, lid, output,
+        base::OnceCallback<void(std::pair<bool, bool>)>());
   }
 }
 

content/browser/webrtc/webrtc_internals.cc

@@ -133,7 +133,8 @@ WebRTCInternals::WebRTCInternals(int aggregate_updates_ms,
         command_line_derived_logging_path_.Append(kEventLogFilename);
     WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
     if (logger) {
-      logger->EnableLocalLogging(local_logs_path);
+      logger->EnableLocalLogging(local_logs_path,
+                                 base::OnceCallback<void(bool)>());
     }
     // For clarity's sake, though these aren't supposed to be regarded now:
     event_log_recordings_ = true;
@@ -376,7 +377,8 @@ void WebRTCInternals::EnableLocalEventLogRecordings(
 #if defined(OS_ANDROID)
   WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
   if (logger) {
-    logger->EnableLocalLogging(event_log_recordings_file_path_);
+    logger->EnableLocalLogging(event_log_recordings_file_path_,
+                               base::OnceCallback<void(bool)>());
   }
 #else
   DCHECK(web_contents);
@@ -397,7 +399,7 @@ void WebRTCInternals::DisableLocalEventLogRecordings() {
   DCHECK(CanToggleEventLogRecordings());
   WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
   if (logger) {
-    logger->DisableLocalLogging();
+    logger->DisableLocalLogging(base::OnceCallback<void(bool)>());
   }
 }
 
@@ -446,7 +448,7 @@ void WebRTCInternals::FileSelected(const base::FilePath& path,
       event_log_recordings_ = true;
       WebRtcEventLogger* const logger = WebRtcEventLogger::Get();
       if (logger) {
-        logger->EnableLocalLogging(path);
+        logger->EnableLocalLogging(path, base::OnceCallback<void(bool)>());
       }
       break;
     }

content/public/browser/webrtc_event_logger.h

@@ -45,9 +45,8 @@ class CONTENT_EXPORT WebRtcEventLogger {
   // This function must not be called for an off-the-records BrowserContext.
   // Local-logging is not associated with BrowserContexts, and is allowed even
   // if EnableForBrowserContext is not called. That is, even for incognito mode.
-  virtual void EnableForBrowserContext(
-      const BrowserContext* browser_context,
-      base::OnceClosure reply = base::OnceClosure()) = 0;
+  virtual void EnableForBrowserContext(const BrowserContext* browser_context,
+                                       base::OnceClosure reply) = 0;
 
   // Disables WebRTC event logging for a given BrowserContext. New remote-bound
   // WebRTC event logs will no longer be created for this BrowserContext.
@@ -55,9 +54,8 @@ class CONTENT_EXPORT WebRtcEventLogger {
   // therefore be careful note to call any of BrowserContext's virtual methods.
   // TODO(eladalon): After changing to a Profile-centered interface, change this
   // to not even receive a pointer. https://crbug.com/775415
-  virtual void DisableForBrowserContext(
-      const BrowserContext* browser_context,
-      base::OnceClosure reply = base::OnceClosure()) = 0;
+  virtual void DisableForBrowserContext(const BrowserContext* browser_context,
+                                        base::OnceClosure reply) = 0;
 
   // Call this to let the logger know when a PeerConnection was created.
   // |peer_connection_id| should be a non-empty, relatively short (i.e.
@@ -71,8 +69,7 @@ class CONTENT_EXPORT WebRtcEventLogger {
   virtual void PeerConnectionAdded(int render_process_id,
                                    int lid,
                                    const std::string& peer_connection_id,
-                                   base::OnceCallback<void(bool)> reply =
-                                       base::OnceCallback<void(bool)>()) = 0;
+                                   base::OnceCallback<void(bool)> reply) = 0;
 
   // Call this to let the logger know when a PeerConnection was closed.
   // If a reply callback is given, it will be posted back to BrowserThread::UI,
@@ -81,8 +78,7 @@ class CONTENT_EXPORT WebRtcEventLogger {
   // or if it has since already been removed).
   virtual void PeerConnectionRemoved(int render_process_id,
                                      int lid,
-                                     base::OnceCallback<void(bool)> reply =
-                                         base::OnceCallback<void(bool)>()) = 0;
+                                     base::OnceCallback<void(bool)> reply) = 0;
 
   // Call this to let the logger know when a PeerConnection was stopped.
   // Closing of a peer connection is an irreversible action. Its distinction
@@ -90,8 +86,7 @@ class CONTENT_EXPORT WebRtcEventLogger {
   // been garbage collected.
   virtual void PeerConnectionStopped(int render_process_id,
                                      int lid,
-                                     base::OnceCallback<void(bool)> reply =
-                                         base::OnceCallback<void(bool)>()) = 0;
+                                     base::OnceCallback<void(bool)> reply) = 0;
 
   // Enable local logging of WebRTC events.
   // Local logging is distinguished from remote logging, in that local logs are
@@ -109,15 +104,13 @@ class CONTENT_EXPORT WebRtcEventLogger {
   // will get a local log file associated (specifically, we do *not* guarantee
   // it would be either the oldest or the newest).
   virtual void EnableLocalLogging(const base::FilePath& base_path,
-                                  base::OnceCallback<void(bool)> reply =
-                                      base::OnceCallback<void(bool)>()) = 0;
+                                  base::OnceCallback<void(bool)> reply) = 0;
 
   // Disable local logging of WebRTC events.
   // Any active local logs are stopped. Peer connections added after this call
   // will not get a local log associated with them (unless local logging is
   // once again enabled).
-  virtual void DisableLocalLogging(base::OnceCallback<void(bool)> reply =
-                                       base::OnceCallback<void(bool)>()) = 0;
+  virtual void DisableLocalLogging(base::OnceCallback<void(bool)> reply) = 0;
 
   // Called when a new log fragment is sent from the renderer. This will
   // potentially be written to a local WebRTC event log, a remote-bound log
@@ -131,8 +124,7 @@ class CONTENT_EXPORT WebRtcEventLogger {
       int render_process_id,
       int lid,
       const std::string& message,
-      base::OnceCallback<void(std::pair<bool, bool>)> reply =
-          base::OnceCallback<void(std::pair<bool, bool>)>()) = 0;
+      base::OnceCallback<void(std::pair<bool, bool>)> reply) = 0;
 
  protected:
   friend WebRTCInternalsIntegrationBrowserTest;  // (PostNullTaskForTesting)