Update cast webview JS channels

The embedder implementation assumes that channels are persistent and
set up ahead of time, so now we track RenderFrameHosts for our
WebContents and dispatch channel events to every frame

Bug: b/141864193
Test: on device with weather
Change-Id: I74182d226fae7d6d63b618aa92f60fadb9516d8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881671Reviewed-by: Daniel Nicoara <dnicoara@chromium.org>
Commit-Queue: Albert Chaulk <achaulk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709971}

chromecast/browser/webview/js_channel_service.cc

@@ -7,6 +7,7 @@
 #include <algorithm>
 
 #include "base/lazy_instance.h"
+#include "base/observer_list.h"
 #include "content/public/browser/render_process_host.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
@@ -27,15 +28,26 @@ class JsChannelImpl : public mojom::JsChannel {
   DISALLOW_COPY_AND_ASSIGN(JsChannelImpl);
 };
 
-struct Instance {
-  int process_id;
-  int routing_id;
-  JsClientInstance* instance;
+// The web contents and channel implementations don't know about each other
+// so keep some global state to allow access.
+struct JsChannelsGlobalState {
+  static JsChannelsGlobalState& Get();
+
+  struct Instance {
+    int process_id;
+    int routing_id;
+    JsClientInstance* instance;
+  };
+  std::vector<Instance> instance_list;
+  base::ObserverList<JsClientInstance::Observer> observer_list;
 };
+static base::LazyInstance<JsChannelsGlobalState>::DestructorAtExit
+    g_global_state = LAZY_INSTANCE_INITIALIZER;
 
-using EndpointList = std::vector<Instance>;
-static base::LazyInstance<EndpointList>::DestructorAtExit g_instance_list =
-    LAZY_INSTANCE_INITIALIZER;
+// static
+JsChannelsGlobalState& JsChannelsGlobalState::Get() {
+  return g_global_state.Get();
+}
 
 }  // namespace
 
@@ -74,19 +86,32 @@ JsClientInstance::JsClientInstance(
       base::BindRepeating([](JsClientInstance* self, uint32_t err,
                              const std::string& str) { delete self; },
                           base::Unretained(this)));
-  g_instance_list.Get().push_back({process_id, routing_id, this});
+  auto& state = JsChannelsGlobalState::Get();
+  state.instance_list.push_back({process_id, routing_id, this});
+  for (auto& o : state.observer_list)
+    o.OnJsClientInstanceRegistered(process_id, routing_id, this);
 }
 
 JsClientInstance::~JsClientInstance() {
-  auto& list = g_instance_list.Get();
+  auto& list = JsChannelsGlobalState::Get().instance_list;
   list.erase(std::find_if(list.begin(), list.end(), [this](const auto& e) {
     return e.instance == this;
   }));
 }
 
+// static
+void JsClientInstance::AddObserver(Observer* observer) {
+  JsChannelsGlobalState::Get().observer_list.AddObserver(observer);
+}
+
+// static
+void JsClientInstance::RemoveObserver(Observer* observer) {
+  JsChannelsGlobalState::Get().observer_list.RemoveObserver(observer);
+}
+
 // static
 JsClientInstance* JsClientInstance::Find(int process_id, int routing_id) {
-  for (auto& e : g_instance_list.Get()) {
+  for (auto& e : JsChannelsGlobalState::Get().instance_list) {
     if (e.process_id == process_id && e.routing_id == routing_id)
       return e.instance;
   }

chromecast/browser/webview/js_channel_service.h

@@ -7,6 +7,7 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/observer_list_types.h"
 #include "chromecast/common/mojom/js_channel.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -44,11 +45,21 @@ class JsChannelService : public mojom::JsChannelBindingProvider {
 
 class JsClientInstance {
  public:
+  class Observer : public base::CheckedObserver {
+   public:
+    virtual void OnJsClientInstanceRegistered(int process_id,
+                                              int routing_id,
+                                              JsClientInstance* instance) = 0;
+  };
+
   void AddChannel(const std::string& channel, JsChannelCallback callback);
   void RemoveChannel(const std::string& channel);
 
   static JsClientInstance* Find(int process_id, int routing_id);
 
+  static void AddObserver(Observer* observer);
+  static void RemoveObserver(Observer* observer);
+
  private:
   friend class JsChannelService;
 

chromecast/browser/webview/web_content_controller.cc

@@ -27,9 +27,11 @@ namespace chromecast {
 
 WebContentController::WebContentController(Client* client) : client_(client) {
   js_channels_ = std::make_unique<WebContentJsChannels>(client_);
+  JsClientInstance::AddObserver(this);
 }
 
 WebContentController::~WebContentController() {
+  JsClientInstance::RemoveObserver(this);
   if (surface_) {
     surface_->RemoveSurfaceObserver(this);
     surface_->SetEmbeddedSurfaceId(base::RepeatingCallback<viz::SurfaceId()>());
@@ -254,41 +256,21 @@ void WebContentController::HandleEvaluateJavascript(
 
 void WebContentController::HandleAddJavascriptChannels(
     const webview::AddJavascriptChannelsRequest& request) {
-  auto* rfh = GetWebContents()->GetMainFrame();
-  if (!rfh) {
-    LOG(WARNING) << "No current RenderFrameHost";
-    return;
-  }
-
-  auto* client =
-      JsClientInstance::Find(rfh->GetProcess()->GetID(), rfh->GetRoutingID());
-  if (!client) {
-    LOG(WARNING) << "Requested to add JS channels but no mojo client found";
-    return;
-  }
-
   for (auto& channel : request.channels()) {
-    client->AddChannel(channel,
-                       base::BindRepeating(&WebContentJsChannels::SendMessage,
-                                           js_channels_->AsWeakPtr()));
+    current_javascript_channel_set_.insert(channel);
+    for (auto* frame : current_render_frame_set_) {
+      ChannelModified(frame, channel, true);
+    }
   }
 }
 
 void WebContentController::HandleRemoveJavascriptChannels(
     const webview::RemoveJavascriptChannelsRequest& request) {
-  auto* rfh = GetWebContents()->GetMainFrame();
-  if (!rfh)
-    return;
-
-  auto* client =
-      JsClientInstance::Find(rfh->GetProcess()->GetID(), rfh->GetRoutingID());
-  if (!client) {
-    LOG(WARNING) << "Requested to remove JS channels but no mojo client found";
-    return;
-  }
-
   for (auto& channel : request.channels()) {
-    client->RemoveChannel(channel);
+    current_javascript_channel_set_.erase(channel);
+    for (auto* frame : current_render_frame_set_) {
+      ChannelModified(frame, channel, false);
+    }
   }
 }
 
@@ -395,6 +377,63 @@ void WebContentController::OnSurfaceDestroying(exo::Surface* surface) {
   surface_ = nullptr;
 }
 
+void WebContentController::RenderFrameCreated(
+    content::RenderFrameHost* render_frame_host) {
+  current_render_frame_set_.insert(render_frame_host);
+  auto* instance =
+      JsClientInstance::Find(render_frame_host->GetProcess()->GetID(),
+                             render_frame_host->GetRoutingID());
+  // If the instance doesn't exist yet the JsClientInstance observer will see
+  // it later on.
+  if (instance)
+    SendInitialChannelSet(instance);
+}
+
+void WebContentController::RenderFrameDeleted(
+    content::RenderFrameHost* render_frame_host) {
+  current_render_frame_set_.erase(render_frame_host);
+}
+
+void WebContentController::OnJsClientInstanceRegistered(
+    int process_id,
+    int routing_id,
+    JsClientInstance* instance) {
+  if (current_render_frame_set_.find(content::RenderFrameHost::FromID(
+          process_id, routing_id)) != current_render_frame_set_.end()) {
+    // If the frame exists in the set then it cannot have been handled by
+    // RenderFrameCreated.
+    SendInitialChannelSet(instance);
+  }
+}
+
+void WebContentController::ChannelModified(content::RenderFrameHost* frame,
+                                           const std::string& channel,
+                                           bool added) {
+  auto* instance = JsClientInstance::Find(frame->GetProcess()->GetID(),
+                                          frame->GetRoutingID());
+  if (instance) {
+    if (added) {
+      instance->AddChannel(channel, GetJsChannelCallback());
+    } else {
+      instance->RemoveChannel(channel);
+    }
+  } else {
+    LOG(WARNING) << "Cannot change channel " << channel << " for "
+                 << frame->GetLastCommittedURL().possibly_invalid_spec();
+  }
+}
+
+JsChannelCallback WebContentController::GetJsChannelCallback() {
+  return base::BindRepeating(&WebContentJsChannels::SendMessage,
+                             js_channels_->AsWeakPtr());
+}
+
+void WebContentController::SendInitialChannelSet(JsClientInstance* instance) {
+  JsChannelCallback callback = GetJsChannelCallback();
+  for (auto& channel : current_javascript_channel_set_)
+    instance->AddChannel(channel, callback);
+}
+
 WebContentJsChannels::WebContentJsChannels(WebContentController::Client* client)
     : client_(client) {}
 

chromecast/browser/webview/web_content_controller.h

@@ -6,12 +6,14 @@
 #define CHROMECAST_BROWSER_WEBVIEW_WEB_CONTENT_CONTROLLER_H_
 
 #include <memory>
+#include <set>
 #include <string>
 
 #include "chromecast/browser/webview/js_channel_service.h"
 #include "chromecast/browser/webview/proto/webview.pb.h"
 #include "components/exo/surface.h"
 #include "components/exo/surface_observer.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "ui/events/gestures/gesture_recognizer_impl.h"
 
 namespace aura {
@@ -19,7 +21,7 @@ class Window;
 }  // namespace aura
 
 namespace content {
-class WebContents;
+class RenderFrameHost;
 }  // namespace content
 
 namespace chromecast {
@@ -27,7 +29,9 @@ namespace chromecast {
 class WebContentJsChannels;
 
 // Processes proto commands to control WebContents
-class WebContentController : public exo::SurfaceObserver {
+class WebContentController : public exo::SurfaceObserver,
+                             public content::WebContentsObserver,
+                             public JsClientInstance::Observer {
  public:
   class Client {
    public:
@@ -47,6 +51,8 @@ class WebContentController : public exo::SurfaceObserver {
   void AttachTo(aura::Window* window, int window_id);
 
  protected:
+  // Subclasses are expected to add/remove this as a WebContentsObserver on
+  // whatever WebContents this manages.
   virtual content::WebContents* GetWebContents() = 0;
   Client* client_;  // Not owned.
   bool has_navigation_delegate_ = false;
@@ -71,14 +77,31 @@ class WebContentController : public exo::SurfaceObserver {
   void HandleSetAutoMediaPlaybackPolicy(
       const webview::SetAutoMediaPlaybackPolicyRequest& request);
   viz::SurfaceId GetSurfaceId();
+  void ChannelModified(content::RenderFrameHost* frame,
+                       const std::string& channel,
+                       bool added);
+  JsChannelCallback GetJsChannelCallback();
+  void SendInitialChannelSet(JsClientInstance* instance);
 
   // exo::SurfaceObserver
   void OnSurfaceDestroying(exo::Surface* surface) override;
 
+  // content::WebContentsObserver
+  void RenderFrameCreated(content::RenderFrameHost* render_frame_host) override;
+  void RenderFrameDeleted(content::RenderFrameHost* render_frame_host) override;
+
+  // JsClientInstance::Observer
+  void OnJsClientInstanceRegistered(int process_id,
+                                    int routing_id,
+                                    JsClientInstance* instance) override;
+
   ui::GestureRecognizerImpl gesture_recognizer_;
 
   exo::Surface* surface_ = nullptr;
 
+  std::set<std::string> current_javascript_channel_set_;
+  std::set<content::RenderFrameHost*> current_render_frame_set_;
+
   base::WeakPtrFactory<WebContentController> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(WebContentController);

chromecast/browser/webview/webview_controller.cc

@@ -60,6 +60,8 @@ WebviewController::WebviewController(content::BrowserContext* browser_context,
       contents_.get(), cast_contents_init);
   cast_web_contents_->AddObserver(this);
 
+  content::WebContentsObserver::Observe(contents_.get());
+
   std::unique_ptr<webview::WebviewResponse> response =
       std::make_unique<webview::WebviewResponse>();
   auto ax_id = contents_->GetMainFrame()->GetAXTreeID().ToString();