Revert "Make ChromeDriver/DevTools APIs return sink structs"

This reverts commit 514383cf.

Reason for revert: suspect causing chrome_all_tast_tests and cros_vm_sanity_test failure on chrome-os-kevin-rel:
https://ci.chromium.org/p/chromium/builders/ci/chromeos-kevin-rel/9026

E.g.
019/05/29 22:45:19 debugd.Printer                           [ FAIL ] Failed to start Chrome: OOBE target not found: context deadline exceeded; last error follows: no chrome://oobe target
...

Original change's description:
> Make ChromeDriver/DevTools APIs return sink structs
> 
> Make the get_sinks API return sink structs instead of sink name strings.
> 
> Bug: 963963
> Change-Id: Idb6b555544f29aaa308b8d289335073348c653f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1615982
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Reviewed-by: John Chen <johnchen@chromium.org>
> Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#664485}

TBR=dgozman@chromium.org,caseq@chromium.org,johnchen@chromium.org,takumif@chromium.org

Change-Id: I9276a8043c250e72f4e2e0bf612cad6a2a892ce2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 963963
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1636667Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#664618}

chrome/browser/devtools/protocol/cast_handler.cc

@@ -17,9 +17,7 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/web_contents.h"
 
-using media_router::MediaRoute;
 using protocol::Response;
-using protocol::Cast::Sink;
 
 namespace {
 
@@ -33,44 +31,42 @@ media_router::MediaRouter* GetMediaRouter(content::WebContents* web_contents) {
 class CastHandler::MediaRoutesObserver
     : public media_router::MediaRoutesObserver {
  public:
-  MediaRoutesObserver(media_router::MediaRouter* router,
-                      base::RepeatingClosure update_callback)
-      : media_router::MediaRoutesObserver(router),
-        update_callback_(std::move(update_callback)) {}
+  explicit MediaRoutesObserver(media_router::MediaRouter* router)
+      : media_router::MediaRoutesObserver(router) {}
   ~MediaRoutesObserver() override = default;
 
-  const std::vector<MediaRoute>& routes() const { return routes_; }
-
- private:
-  void OnRoutesUpdated(
-      const std::vector<MediaRoute>& routes,
-      const std::vector<MediaRoute::Id>& joinable_route_ids) override {
+  void OnRoutesUpdated(const std::vector<media_router::MediaRoute>& routes,
+                       const std::vector<media_router::MediaRoute::Id>&
+                           joinable_route_ids) override {
     routes_ = routes;
-    update_callback_.Run();
   }
 
-  std::vector<MediaRoute> routes_;
-  base::RepeatingClosure update_callback_;
+  const std::vector<media_router::MediaRoute>& routes() const {
+    return routes_;
+  }
+
+ private:
+  std::vector<media_router::MediaRoute> routes_;
 };
 
 class CastHandler::IssuesObserver : public media_router::IssuesObserver {
  public:
-  IssuesObserver(
+  explicit IssuesObserver(
       media_router::MediaRouter* router,
-      base::RepeatingCallback<void(const std::string& issue)> update_callback)
+      base::RepeatingCallback<void(const std::string& issue)> callback)
       : media_router::IssuesObserver(router->GetIssueManager()),
-        update_callback_(std::move(update_callback)) {
+        callback_(std::move(callback)) {
     Init();
   }
   ~IssuesObserver() override = default;
 
   void OnIssue(const media_router::Issue& issue) override {
-    update_callback_.Run(issue.info().title);
+    callback_.Run(issue.info().title);
   }
-  void OnIssuesCleared() override { update_callback_.Run(std::string()); }
+  void OnIssuesCleared() override { callback_.Run(std::string()); }
 
  private:
-  base::RepeatingCallback<void(const std::string& issue)> update_callback_;
+  base::RepeatingCallback<void(const std::string& issue)> callback_;
 };
 
 CastHandler::CastHandler(content::WebContents* web_contents,
@@ -120,7 +116,7 @@ Response CastHandler::StopCasting(const std::string& in_sink_name) {
   const media_router::MediaSink::Id& sink_id = GetSinkIdByName(in_sink_name);
   if (sink_id.empty())
     return Response::Error("Sink not found");
-  const MediaRoute::Id& route_id = GetRouteIdForSink(sink_id);
+  const media_router::MediaRoute::Id& route_id = GetRouteIdForSink(sink_id);
   if (route_id.empty())
     return Response::Error("Route not found");
   router_->TerminateRoute(route_id);
@@ -131,6 +127,7 @@ Response CastHandler::StopCasting(const std::string& in_sink_name) {
 Response CastHandler::Enable(protocol::Maybe<std::string> in_presentation_url) {
   EnsureInitialized();
   StartObservingForSinks(std::move(in_presentation_url));
+  StartObservingForIssues();
   return Response::OK();
 }
 
@@ -138,15 +135,21 @@ Response CastHandler::Disable() {
   query_result_manager_.reset();
   routes_observer_.reset();
   issues_observer_.reset();
-  for (const MediaRoute::Id& route_id : initiated_routes_)
+  for (const media_router::MediaRoute::Id& route_id : initiated_routes_)
     router_->TerminateRoute(route_id);
   return Response::OK();
 }
 
 void CastHandler::OnResultsUpdated(
     const std::vector<media_router::MediaSinkWithCastModes>& sinks) {
+  std::unique_ptr<protocol::Array<std::string>> sink_names =
+      protocol::Array<std::string>::create();
+  for (const media_router::MediaSinkWithCastModes& sink_with_modes : sinks)
+    sink_names->addItem(sink_with_modes.sink.name());
+
+  if (frontend_)
+    frontend_->SinksUpdated(std::move(sink_names));
   sinks_ = sinks;
-  SendSinkUpdate();
 }
 
 CastHandler::CastHandler(content::WebContents* web_contents)
@@ -155,18 +158,14 @@ CastHandler::CastHandler(content::WebContents* web_contents)
       weak_factory_(this) {}
 
 void CastHandler::EnsureInitialized() {
-  if (query_result_manager_)
+  if (initialized_)
     return;
 
   query_result_manager_ =
       std::make_unique<media_router::QueryResultManager>(router_);
   query_result_manager_->AddObserver(this);
-  routes_observer_ = std::make_unique<MediaRoutesObserver>(
-      router_, base::BindRepeating(&CastHandler::SendSinkUpdate,
-                                   base::Unretained(this)));
-  issues_observer_ = std::make_unique<IssuesObserver>(
-      router_,
-      base::BindRepeating(&CastHandler::OnIssue, base::Unretained(this)));
+  routes_observer_ = std::make_unique<MediaRoutesObserver>(router_);
+  initialized_ = true;
 }
 
 void CastHandler::StartPresentation(
@@ -203,17 +202,18 @@ media_router::MediaSink::Id CastHandler::GetSinkIdByName(
       [&sink_name](const media_router::MediaSinkWithCastModes& sink) {
         return sink.sink.name() == sink_name;
       });
-  return it == sinks_.end() ? media_router::MediaSink::Id() : it->sink.id();
+  return it == sinks_.end() ? media_router::MediaSink::Id() : (*it).sink.id();
 }
 
-MediaRoute::Id CastHandler::GetRouteIdForSink(
+media_router::MediaRoute::Id CastHandler::GetRouteIdForSink(
     const media_router::MediaSink::Id& sink_id) const {
   const auto& routes = routes_observer_->routes();
   auto it = std::find_if(routes.begin(), routes.end(),
-                         [&sink_id](const MediaRoute& route) {
+                         [&sink_id](const media_router::MediaRoute& route) {
                            return route.media_sink_id() == sink_id;
                          });
-  return it == routes.end() ? MediaRoute::Id() : it->media_route_id();
+  return it == routes.end() ? media_router::MediaRoute::Id()
+                            : (*it).media_route_id();
 }
 
 void CastHandler::StartObservingForSinks(
@@ -234,31 +234,10 @@ void CastHandler::StartObservingForSinks(
   }
 }
 
-void CastHandler::SendSinkUpdate() {
-  if (!frontend_)
-    return;
-
-  std::unique_ptr<protocol::Array<Sink>> protocol_sinks =
-      protocol::Array<Sink>::create();
-  for (const media_router::MediaSinkWithCastModes& sink_with_modes : sinks_) {
-    auto route_it = std::find_if(
-        routes_observer_->routes().begin(), routes_observer_->routes().end(),
-        [&sink_with_modes](const MediaRoute& route) {
-          return route.media_sink_id() == sink_with_modes.sink.id();
-        });
-    std::string session = route_it == routes_observer_->routes().end()
-                              ? ""
-                              : route_it->description();
-    std::unique_ptr<Sink> sink = Sink::Create()
-                                     .SetName(sink_with_modes.sink.name())
-                                     .SetId(sink_with_modes.sink.id())
-                                     .Build();
-    if (!session.empty())
-      sink->SetSession(session);
-
-    protocol_sinks->addItem(std::move(sink));
-  }
-  frontend_->SinksUpdated(std::move(protocol_sinks));
+void CastHandler::StartObservingForIssues() {
+  issues_observer_ = std::make_unique<IssuesObserver>(
+      router_,
+      base::BindRepeating(&CastHandler::OnIssue, base::Unretained(this)));
 }
 
 void CastHandler::OnTabMirroringStarted(

chrome/browser/devtools/protocol/cast_handler.h

@@ -69,10 +69,7 @@ class CastHandler : public protocol::Cast::Backend,
       const media_router::MediaSink::Id& sink_id) const;
 
   void StartObservingForSinks(protocol::Maybe<std::string> presentation_url);
-
-  // Sends a notification that sinks (or their associated routes) have been
-  // updated.
-  void SendSinkUpdate();
+  void StartObservingForIssues();
 
   void OnTabMirroringStarted(
       std::unique_ptr<StartTabMirroringCallback> callback,
@@ -99,6 +96,8 @@ class CastHandler : public protocol::Cast::Backend,
 
   std::unique_ptr<protocol::Cast::Frontend> frontend_;
 
+  bool initialized_ = false;
+
   base::WeakPtrFactory<CastHandler> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(CastHandler);

chrome/test/chromedriver/chrome/cast_tracker.cc

@@ -21,7 +21,7 @@ Status CastTracker::OnEvent(DevToolsClient* client,
                             const std::string& method,
                             const base::DictionaryValue& params) {
   if (method == "Cast.sinksUpdated") {
-    const base::Value* sinks = params.FindKey("sinks");
+    const base::Value* sinks = params.FindKey("sinkNames");
     if (sinks)
       sinks_ = sinks->Clone();
   } else if (method == "Cast.issueUpdated") {

chrome/test/chromedriver/chrome/cast_tracker_unittest.cc

@@ -15,14 +15,6 @@ using testing::Return;
 
 namespace {
 
-base::Value CreateSink(const std::string& name, const std::string& id) {
-  base::Value sink(base::Value::Type::DICTIONARY);
-  sink.SetKey("name", base::Value(name));
-  sink.SetKey("id", base::Value(id));
-  sink.SetKey("session", base::Value("Example session"));
-  return sink;
-}
-
 class MockDevToolsClient : public StubDevToolsClient {
  public:
   MOCK_METHOD2(SendCommand,
@@ -54,13 +46,13 @@ TEST_F(CastTrackerTest, OnSinksUpdated) {
   EXPECT_EQ(0u, cast_tracker_->sinks().GetList().size());
 
   base::Value sinks(base::Value::Type::LIST);
-  sinks.GetList().emplace_back(CreateSink("sink1", "1"));
-  sinks.GetList().emplace_back(CreateSink("sink2", "2"));
-  params.SetKey("sinks", std::move(sinks));
+  sinks.GetList().emplace_back(base::Value("sink1"));
+  sinks.GetList().emplace_back(base::Value("sink2"));
+  params.SetKey("sinkNames", std::move(sinks));
   cast_tracker_->OnEvent(&devtools_client_, "Cast.sinksUpdated", params);
   EXPECT_EQ(2u, cast_tracker_->sinks().GetList().size());
 
-  params.SetKey("sinks", base::Value(base::Value::Type::LIST));
+  params.SetKey("sinkNames", base::Value(base::Value::Type::LIST));
   cast_tracker_->OnEvent(&devtools_client_, "Cast.sinksUpdated", params);
   EXPECT_EQ(0u, cast_tracker_->sinks().GetList().size());
 }

chrome/test/chromedriver/window_commands.h

@@ -15,7 +15,7 @@
 namespace base {
 class DictionaryValue;
 class Value;
-}  // namespace base
+}
 
 struct Session;
 class Status;
@@ -26,8 +26,7 @@ typedef base::Callback<Status(Session* session,
                               WebView* web_view,
                               const base::DictionaryValue&,
                               std::unique_ptr<base::Value>*,
-                              Timeout*)>
-    WindowCommand;
+                              Timeout*)> WindowCommand;
 
 // Execute a Window Command on the target window.
 Status ExecuteWindowCommand(const WindowCommand& command,
@@ -427,7 +426,7 @@ Status ExecuteStopCasting(Session* session,
                           std::unique_ptr<base::Value>* value,
                           Timeout* timeout);
 
-// Returns a list of Cast sinks that are available.
+// Returns a list of names of Cast sinks that are available.
 Status ExecuteGetSinks(Session* session,
                        WebView* web_view,
                        const base::DictionaryValue& params,

third_party/blink/renderer/core/inspector/browser_protocol.pdl

@@ -1343,14 +1343,6 @@ experimental domain CacheStorage
 # functionalities.
 experimental domain Cast
 
-  type Sink extends object
-    properties
-      string name
-      string id
-      # Text describing the current session. Present only if there is an active
-      # session on the sink.
-      optional string session
-
   # Starts observing for sinks that can be used for tab mirroring, and if set,
   # sinks compatible with |presentationUrl| as well. When sinks are found, a
   # |sinksUpdated| event is fired.
@@ -1383,7 +1375,7 @@ experimental domain Cast
   # device or a software surface that you can cast to.
   event sinksUpdated
     parameters
-      array of Sink sinks
+      array of string sinkNames
 
   # This is fired whenever the outstanding issue/error message changes.
   # |issueMessage| is empty if there is no issue.