Revert "[Media Router] Add AppAvailability Logs"

This reverts commit 9f54bb86.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1114308

Original change's description:
> [Media Router] Add AppAvailability Logs
> 
> This CL adds a mojo::Remote<mojom::Logger> in CastAppDiscoveryService to
> log unavailable (app_id, sink_id) pairs and socket not found errors.
> 
> Bug: 687380, b/154273698
> Change-Id: Ib50c74cd9a0e66c9c05b3f36fdf653e2565d8048
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332329
> Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
> Commit-Queue: Muyao Xu <muyaoxu@google.com>
> Cr-Commit-Position: refs/heads/master@{#795981}

TBR=takumif@chromium.org,muyaoxu@google.com

Change-Id: I28d8206ee3b243ff82d210d561869e7eb4dc04e1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 687380
Bug: b/154273698
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343764Reviewed-by: Roberto Carrillo <robertocn@chromium.org>
Commit-Queue: Roberto Carrillo <robertocn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796102}

chrome/browser/media/router/mojo/media_router_desktop_unittest.cc

@@ -72,11 +72,9 @@ class MediaRouterDesktopTest : public MediaRouterMojoTest {
     feature_list_.InitAndDisableFeature(kDialMediaRouteProvider);
     cast_media_sink_service = std::make_unique<MockCastMediaSinkService>();
     cast_media_sink_service_ = cast_media_sink_service.get();
-    media_sink_service_ =
-        std::unique_ptr<DualMediaSinkService>(new DualMediaSinkService(
-            std::move(cast_media_sink_service),
-            std::make_unique<MockDialMediaSinkService>(),
-            std::make_unique<MockCastAppDiscoveryService>()));
+    media_sink_service_ = std::unique_ptr<DualMediaSinkService>(
+        new DualMediaSinkService(std::move(cast_media_sink_service),
+                                 std::make_unique<MockDialMediaSinkService>()));
     return std::unique_ptr<MediaRouterDesktop>(
         new MediaRouterDesktop(profile(), media_sink_service_.get()));
   }

chrome/browser/media/router/providers/cast/cast_app_discovery_service.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/media/router/providers/cast/cast_app_discovery_service.h"
 
 #include "base/bind.h"
-#include "base/strings/strcat.h"
 #include "base/time/tick_clock.h"
 #include "chrome/browser/media/router/providers/cast/cast_media_route_provider_metrics.h"
 #include "components/cast_channel/cast_message_handler.h"
@@ -16,8 +15,6 @@ namespace media_router {
 
 namespace {
 
-constexpr char kLoggerComponent[] = "CastAppDiscoveryService";
-
 // The minimum time that must elapse before an app availability result can be
 // force refreshed.
 static constexpr base::TimeDelta kRefreshThreshold =
@@ -82,15 +79,7 @@ CastAppDiscoveryServiceImpl::StartObservingMediaSinks(
         cast_channel::CastSocket* socket =
             socket_service_->GetSocket(channel_id);
         if (!socket) {
-          if (logger_.is_bound()) {
-            logger_->LogError(
-                mojom::LogCategory::kDiscovery, kLoggerComponent,
-                base::StringPrintf(
-                    "Socket not found for channel id: %d when starting "
-                    "discovery for source.",
-                    channel_id),
-                sink.first, source.source_id(), "");
-          }
+          DVLOG(1) << "Socket not found for id " << channel_id;
           continue;
         }
 
@@ -109,36 +98,18 @@ void CastAppDiscoveryServiceImpl::Refresh() {
   // being iterated.
   for (const auto& sink : sinks) {
     for (const auto& app_id : app_ids) {
-      int channel_id = sink.second.cast_data().cast_channel_id;
-      cast_channel::CastSocket* socket = socket_service_->GetSocket(channel_id);
-      if (!socket) {
-        if (logger_.is_bound()) {
-          logger_->LogError(
-              mojom::LogCategory::kDiscovery, kLoggerComponent,
-              base::StringPrintf(
-                  "Socket not found for channel id: %d when refreshing "
-                  "the discovery state.",
-                  channel_id),
-              sink.first, "", "");
+        int channel_id = sink.second.cast_data().cast_channel_id;
+        cast_channel::CastSocket* socket =
+            socket_service_->GetSocket(channel_id);
+        if (!socket) {
+          DVLOG(1) << "Socket not found for id " << channel_id;
+          continue;
         }
-        continue;
+        RequestAppAvailability(socket, app_id, sink.first);
       }
-      RequestAppAvailability(socket, app_id, sink.first);
-    }
   }
 }
 
-void CastAppDiscoveryServiceImpl::BindLogger(
-    mojo::PendingRemote<mojom::Logger> pending_remote) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  logger_.Bind(std::move(pending_remote));
-}
-
-scoped_refptr<base::SequencedTaskRunner>
-CastAppDiscoveryServiceImpl::task_runner() {
-  return socket_service_->task_runner();
-}
-
 void CastAppDiscoveryServiceImpl::MaybeRemoveSinkQueryEntry(
     const CastMediaSource& source) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -203,13 +174,8 @@ void CastAppDiscoveryServiceImpl::UpdateAppAvailability(
   if (!media_sink_service_->GetSinkById(sink_id))
     return;
 
-  if (availability != cast_channel::GetAppAvailabilityResult::kAvailable &&
-      logger_.is_bound()) {
-    logger_->LogInfo(
-        mojom::LogCategory::kDiscovery, kLoggerComponent,
-        base::StrCat({"App ", app_id, " on sink is ", ToString(availability)}),
-        sink_id, "", "");
-  }
+  DVLOG(1) << "App " << app_id << " on sink " << sink_id << " is "
+           << ToString(availability);
 
   UpdateSinkQueries(availability_tracker_.UpdateAppAvailability(
       sink_id, app_id, {availability, clock_->NowTicks()}));

chrome/browser/media/router/providers/cast/cast_app_discovery_service.h

@@ -20,10 +20,8 @@
 #include "chrome/common/media_router/discovery/media_sink_service_base.h"
 #include "chrome/common/media_router/media_sink.h"
 #include "chrome/common/media_router/media_source.h"
-#include "chrome/common/media_router/mojom/logger.mojom.h"
 #include "chrome/common/media_router/providers/cast/cast_media_source.h"
 #include "components/cast_channel/cast_message_util.h"
-#include "mojo/public/cpp/bindings/remote.h"
 
 namespace base {
 class TickClock;
@@ -60,14 +58,6 @@ class CastAppDiscoveryService {
   // this method when the user initiates a user gesture (such as opening the
   // Media Router dialog).
   virtual void Refresh() = 0;
-
-  // Binds |pending_remote| to the Mojo Remote owned by |impl_|.
-  virtual void BindLogger(
-      mojo::PendingRemote<mojom::Logger> pending_remote) = 0;
-
-  // Returns the SequencedTaskRunner that should be used to invoke methods on
-  // this instance. Can be invoked on any thread.
-  virtual scoped_refptr<base::SequencedTaskRunner> task_runner() = 0;
 };
 
 // Keeps track of sink queries and listens to CastMediaSinkServiceImpl for sink
@@ -92,10 +82,6 @@ class CastAppDiscoveryServiceImpl : public CastAppDiscoveryService,
   // pairs whose status is kUnavailable or kUnknown.
   void Refresh() override;
 
-  void BindLogger(mojo::PendingRemote<mojom::Logger> pending_remote) override;
-
-  scoped_refptr<base::SequencedTaskRunner> task_runner() override;
-
  private:
   friend class CastAppDiscoveryServiceImplTest;
 
@@ -150,10 +136,6 @@ class CastAppDiscoveryServiceImpl : public CastAppDiscoveryService,
   CastAppAvailabilityTracker availability_tracker_;
 
   const base::TickClock* const clock_;
-  // Mojo Remote to the logger owned by the Media Router. The Remote is not
-  // bound until |BindLogger()| is called. Always check if |logger_.is_bound()|
-  // is true before using.
-  mojo::Remote<mojom::Logger> logger_;
 
   SEQUENCE_CHECKER(sequence_checker_);
   base::WeakPtrFactory<CastAppDiscoveryServiceImpl> weak_ptr_factory_{this};

chrome/browser/media/router/providers/cast/dual_media_sink_service.cc

@@ -89,11 +89,9 @@ DualMediaSinkService::DualMediaSinkService() {
 
 DualMediaSinkService::DualMediaSinkService(
     std::unique_ptr<CastMediaSinkService> cast_media_sink_service,
-    std::unique_ptr<DialMediaSinkService> dial_media_sink_service,
-    std::unique_ptr<CastAppDiscoveryService> cast_app_discovery_service)
+    std::unique_ptr<DialMediaSinkService> dial_media_sink_service)
     : dial_media_sink_service_(std::move(dial_media_sink_service)),
-      cast_media_sink_service_(std::move(cast_media_sink_service)),
-      cast_app_discovery_service_(std::move(cast_app_discovery_service)) {}
+      cast_media_sink_service_(std::move(cast_media_sink_service)) {}
 
 DualMediaSinkService::~DualMediaSinkService() = default;
 
@@ -118,15 +116,6 @@ void DualMediaSinkService::BindLogger(LoggerImpl* logger_impl) {
   mojo::PendingRemote<mojom::Logger> dial_pending_remote;
   logger_impl->Bind(dial_pending_remote.InitWithNewPipeAndPassReceiver());
   dial_media_sink_service_->BindLogger(std::move(dial_pending_remote));
-
-  mojo::PendingRemote<mojom::Logger> cast_discovery_pending_remote;
-  logger_impl->Bind(
-      cast_discovery_pending_remote.InitWithNewPipeAndPassReceiver());
-  cast_app_discovery_service_->task_runner()->PostTask(
-      FROM_HERE,
-      base::BindOnce(&CastAppDiscoveryService::BindLogger,
-                     base::Unretained(cast_app_discovery_service_.get()),
-                     std::move(cast_discovery_pending_remote)));
 }
 
 }  // namespace media_router

chrome/browser/media/router/providers/cast/dual_media_sink_service.h

@@ -77,8 +77,7 @@ class DualMediaSinkService {
   // |dial_media_sink_service_|.
   // The binding should be done once and the method is a no-op after the first
   // call.
-  // Marked virtual for testing.
-  virtual void BindLogger(LoggerImpl* logger_impl);
+  void BindLogger(LoggerImpl* logger_impl);
 
   virtual void OnUserGesture();
 
@@ -90,8 +89,7 @@ class DualMediaSinkService {
   // Used by tests.
   DualMediaSinkService(
       std::unique_ptr<CastMediaSinkService> cast_media_sink_service,
-      std::unique_ptr<DialMediaSinkService> dial_media_sink_service,
-      std::unique_ptr<CastAppDiscoveryService> cast_app_discovery_service);
+      std::unique_ptr<DialMediaSinkService> dial_media_sink_service);
   virtual ~DualMediaSinkService();
 
  private:

chrome/browser/media/router/providers/cast/dual_media_sink_service_unittest.cc

@@ -16,15 +16,11 @@ class DualMediaSinkServiceTest : public testing::Test {
   DualMediaSinkServiceTest() {
     auto cast_media_sink_service = std::make_unique<MockCastMediaSinkService>();
     auto dial_media_sink_service = std::make_unique<MockDialMediaSinkService>();
-    auto cast_app_discovery_service =
-        std::make_unique<MockCastAppDiscoveryService>();
     cast_media_sink_service_ = cast_media_sink_service.get();
     dial_media_sink_service_ = dial_media_sink_service.get();
-    cast_app_discovery_service_ = cast_app_discovery_service.get();
     dual_media_sink_service_ = std::unique_ptr<DualMediaSinkService>(
         new DualMediaSinkService(std::move(cast_media_sink_service),
-                                 std::move(dial_media_sink_service),
-                                 std::move(cast_app_discovery_service)));
+                                 std::move(dial_media_sink_service)));
   }
 
   ~DualMediaSinkServiceTest() override = default;
@@ -46,7 +42,6 @@ class DualMediaSinkServiceTest : public testing::Test {
  private:
   MockCastMediaSinkService* cast_media_sink_service_;
   MockDialMediaSinkService* dial_media_sink_service_;
-  MockCastAppDiscoveryService* cast_app_discovery_service_;
   std::unique_ptr<DualMediaSinkService> dual_media_sink_service_;
 };
 

chrome/browser/media/router/test/noop_dual_media_sink_service.cc

@@ -6,14 +6,12 @@
 
 #include "chrome/browser/media/router/discovery/dial/dial_media_sink_service.h"
 #include "chrome/browser/media/router/discovery/mdns/cast_media_sink_service.h"
-#include "chrome/browser/media/router/providers/cast/cast_app_discovery_service.h"
 
 namespace media_router {
 
 NoopDualMediaSinkService::NoopDualMediaSinkService()
     : DualMediaSinkService(std::unique_ptr<CastMediaSinkService>(nullptr),
-                           std::unique_ptr<DialMediaSinkService>(nullptr),
-                           std::unique_ptr<CastAppDiscoveryService>(nullptr)) {}
+                           std::unique_ptr<DialMediaSinkService>(nullptr)) {}
 NoopDualMediaSinkService::~NoopDualMediaSinkService() = default;
 
 }  // namespace media_router

chrome/browser/media/router/test/test_helper.cc

@@ -12,8 +12,6 @@
 
 #if !defined(OS_ANDROID)
 #include "base/json/json_reader.h"
-#include "content/public/browser/browser_task_traits.h"
-#include "content/public/browser/browser_thread.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "url/gurl.h"
 #endif
@@ -40,7 +38,7 @@ MockPresentationConnectionProxy::MockPresentationConnectionProxy() {}
 MockPresentationConnectionProxy::~MockPresentationConnectionProxy() {}
 
 #if !defined(OS_ANDROID)
-MockDialMediaSinkService::MockDialMediaSinkService() = default;
+MockDialMediaSinkService::MockDialMediaSinkService() : DialMediaSinkService() {}
 MockDialMediaSinkService::~MockDialMediaSinkService() = default;
 
 MockCastMediaSinkService::MockCastMediaSinkService() : CastMediaSinkService() {}
@@ -56,10 +54,6 @@ MockCastAppDiscoveryService::StartObservingMediaSinks(
   DoStartObservingMediaSinks(source);
   return callbacks_.Add(callback);
 }
-scoped_refptr<base::SequencedTaskRunner>
-MockCastAppDiscoveryService::task_runner() {
-  return base::CreateSingleThreadTaskRunner({content::BrowserThread::IO});
-}
 
 MockDialAppDiscoveryService::MockDialAppDiscoveryService() = default;
 

chrome/browser/media/router/test/test_helper.h

@@ -28,7 +28,6 @@
 #include "chrome/browser/media/router/providers/dial/dial_activity_manager.h"
 #include "chrome/browser/media/router/providers/dial/dial_internal_message_util.h"
 #include "chrome/common/media_router/discovery/media_sink_internal.h"
-#include "chrome/common/media_router/mojom/logger.mojom.h"
 #include "net/base/ip_endpoint.h"
 #include "services/network/test/test_url_loader_factory.h"
 #endif  // !defined(OS_ANDROID)
@@ -118,10 +117,8 @@ class MockCastAppDiscoveryService : public CastAppDiscoveryService {
   Subscription StartObservingMediaSinks(
       const CastMediaSource& source,
       const SinkQueryCallback& callback) override;
-  scoped_refptr<base::SequencedTaskRunner> task_runner() override;
   MOCK_METHOD1(DoStartObservingMediaSinks, void(const CastMediaSource&));
   MOCK_METHOD0(Refresh, void());
-  MOCK_METHOD1(BindLogger, void(mojo::PendingRemote<mojom::Logger>));
 
   SinkQueryCallbackList& callbacks() { return callbacks_; }