[chrome.displaySource] Session notification improvements

This patch provides several fixes and improvements (simplifications)
to the session life cycle notification mechanism:
1) The 'startSession'/'terminateSession' call completion callbacks are
invoked when the session is actually started/terminated and in accordance
with 'onSessionStarted'/'onSessionTerminated' events propagating (and as
per documentation).
2) The notification from the required sink is filtered out in browser process
avoiding unneeded IPC calls
3) Methods in mojo interface are renamed in order to improve readability

BUG=242107

Review URL: https://codereview.chromium.org/1674583002

Cr-Commit-Position: refs/heads/master@{#374929}

5 files changed, 128 insertions, 64 deletions

diff --git a/extensions/renderer/api/display_source/wifi_display/wifi_display_session.cc b/extensions/renderer/api/display_source/wifi_display/wifi_display_session.cc
index 67edf4b..f6d1104 100644
--- a/extensions/renderer/api/display_source/wifi_display/wifi_display_session.cc
+++ b/extensions/renderer/api/display_source/wifi_display/wifi_display_session.cc
@@ -42,6 +42,8 @@ void WiFiDisplaySession::Start() {
   DCHECK(state_ == DisplaySourceSession::Idle);
   service_->Connect(params_.sink_id, params_.auth_method, params_.auth_data);
   state_ = DisplaySourceSession::Establishing;
+  if (!started_callback_.is_null())
+    started_callback_.Run(params_.sink_id);
 }
 
 void WiFiDisplaySession::Terminate() {
@@ -61,35 +63,26 @@ void WiFiDisplaySession::Terminate() {
   }
 }
 
-void WiFiDisplaySession::OnConnected(
-    int32_t sink_id, const mojo::String& ip_address) {
-  if (sink_id == params_.sink_id) {
-    DCHECK(state_ != DisplaySourceSession::Established);
-    ip_address_ = ip_address;
-    state_ = DisplaySourceSession::Established;
-  }
-
-  if (!started_callback_.is_null())
-    started_callback_.Run(sink_id);
+void WiFiDisplaySession::OnEstablished(const mojo::String& ip_address) {
+  DCHECK(state_ != DisplaySourceSession::Established);
+  ip_address_ = ip_address;
+  state_ = DisplaySourceSession::Established;
 }
 
-void WiFiDisplaySession::OnDisconnected(int32_t sink_id) {
-  if (sink_id == params_.sink_id) {
-    DCHECK(state_ == DisplaySourceSession::Established ||
-           state_ == DisplaySourceSession::Terminating);
-    state_ = DisplaySourceSession::Idle;
-  }
-
+void WiFiDisplaySession::OnTerminated() {
+  DCHECK(state_ != DisplaySourceSession::Idle);
+  state_ = DisplaySourceSession::Idle;
   if (!terminated_callback_.is_null())
-    terminated_callback_.Run(sink_id);
+    terminated_callback_.Run(params_.sink_id);
 }
 
-void WiFiDisplaySession::OnError(
-    int32_t sink_id, int32_t type, const mojo::String& description) {
+void WiFiDisplaySession::OnError(int32_t type,
+                                 const mojo::String& description) {
   DCHECK(type > api::display_source::ERROR_TYPE_NONE
          && type <= api::display_source::ERROR_TYPE_LAST);
   if (!error_callback_.is_null())
-    error_callback_.Run(sink_id, static_cast<ErrorType>(type), description);
+    error_callback_.Run(params_.sink_id, static_cast<ErrorType>(type),
+                        description);
 }
 
 void WiFiDisplaySession::OnMessage(const mojo::String& data) {
@@ -103,8 +96,7 @@ void WiFiDisplaySession::OnConnectionError() {
                         kErrorInternal);
   }
 
-  if (state_ == DisplaySourceSession::Established ||
-      state_ == DisplaySourceSession::Terminating) {
+  if (state_ != DisplaySourceSession::Idle) {
     // We must explicitly notify the session termination as it will never
     // arrive from browser process (IPC is broken).
     if (!terminated_callback_.is_null())
diff --git a/extensions/renderer/api/display_source/wifi_display/wifi_display_session.h b/extensions/renderer/api/display_source/wifi_display/wifi_display_session.h
index b807993..5d10308 100644
--- a/extensions/renderer/api/display_source/wifi_display/wifi_display_session.h
+++ b/extensions/renderer/api/display_source/wifi_display/wifi_display_session.h
@@ -26,12 +26,9 @@ class WiFiDisplaySession: public DisplaySourceSession,
   void Terminate() override;
 
   // WiFiDisplaySessionServiceClient overrides.
-  void OnConnected(int32_t sink_id,
-                   const mojo::String& ip_address) override;
-  void OnDisconnected(int32_t sink_id) override;
-  void OnError(int32_t sink_id,
-               int32_t type,
-               const mojo::String& description) override;
+  void OnEstablished(const mojo::String& ip_address) override;
+  void OnTerminated() override;
+  void OnError(int32_t type, const mojo::String& description) override;
   void OnMessage(const mojo::String& data) override;
 
   // A connection error handler for the mojo objects used in this class.
diff --git a/extensions/renderer/display_source_custom_bindings.cc b/extensions/renderer/display_source_custom_bindings.cc
index 50752c0..ab68553 100644
--- a/extensions/renderer/display_source_custom_bindings.cc
+++ b/extensions/renderer/display_source_custom_bindings.cc
@@ -71,8 +71,10 @@ v8::Local<v8::Value> GetChildValue(v8::Local<v8::Object> value,
 
 void DisplaySourceCustomBindings::StartSession(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  CHECK_EQ(1, args.Length());
+  CHECK_EQ(2, args.Length());
   CHECK(args[0]->IsObject());
+  CHECK(args[1]->IsFunction());
+
   v8::Isolate* isolate = context()->isolate();
   v8::Local<v8::Object> start_info = args[0].As<v8::Object>();
 
@@ -164,14 +166,20 @@ void DisplaySourceCustomBindings::StartSession(
   session->SetCallbacks(on_started_callback,
                         on_terminated_callback,
                         on_error_callback);
+
+  CallbackInfo cb_info = GetCallbackInfo(kStarted, sink_id);
+  args.GetReturnValue().Set(static_cast<int32_t>(cb_info.call_id));
+  callbacks_.push_back(cb_info);
+
   session->Start();
   session_map_.insert(std::make_pair(sink_id, std::move(session)));
 }
 
 void DisplaySourceCustomBindings::TerminateSession(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  CHECK_EQ(1, args.Length());
+  CHECK_EQ(2, args.Length());
   CHECK(args[0]->IsInt32());
+  CHECK(args[1]->IsFunction());
 
   v8::Isolate* isolate = context()->isolate();
   int sink_id = args[0]->ToInt32(args.GetIsolate())->Value();
@@ -187,17 +195,40 @@ void DisplaySourceCustomBindings::TerminateSession(
         isolate, kSessionAlreadyTerminating)));
     return;
   }
+
+  CallbackInfo cb_info = GetCallbackInfo(kTerminated, sink_id);
+  args.GetReturnValue().Set(static_cast<int32_t>(cb_info.call_id));
+  callbacks_.push_back(cb_info);
+
   // The session will get removed from session_map_ in OnSessionTerminated.
   session->Terminate();
 }
 
-void DisplaySourceCustomBindings::DispatchSessionStarted(int sink_id) const {
+void DisplaySourceCustomBindings::CallCompletionCallback(
+    int sink_id,
+    CallbackType type,
+    const std::string& error_message) {
+  auto predicate = [sink_id, type](const CallbackInfo& info) -> bool {
+    return info.sink_id == sink_id && info.type == type;
+  };
+  auto it = std::find_if(callbacks_.begin(), callbacks_.end(), predicate);
+  if (it == callbacks_.end())
+    return;
+
   v8::Isolate* isolate = context()->isolate();
+  ModuleSystem* module_system = context()->module_system();
   v8::HandleScope handle_scope(isolate);
   v8::Context::Scope context_scope(context()->v8_context());
-  v8::Local<v8::Array> event_args = v8::Array::New(isolate, 1);
-  event_args->Set(0, v8::Integer::New(isolate, sink_id));
-  context()->DispatchEvent("displaySource.onSessionStarted", event_args);
+
+  v8::Local<v8::Value> callback_args[2];
+  callback_args[0] = v8::Integer::New(isolate, it->call_id);
+  if (error_message.empty())
+    callback_args[1] = v8::Null(isolate);
+  else
+    callback_args[1] = v8::String::NewFromUtf8(isolate, error_message.c_str());
+
+  module_system->CallModuleMethod("displaySource", "callCompletionCallback", 2,
+                                  callback_args);
 }
 
 void DisplaySourceCustomBindings::DispatchSessionTerminated(int sink_id) const {
@@ -242,7 +273,7 @@ DisplaySourceSession* DisplaySourceCustomBindings::GetDisplaySession(
 }
 
 void DisplaySourceCustomBindings::OnSessionStarted(int sink_id) {
-  DispatchSessionStarted(sink_id);
+  CallCompletionCallback(sink_id, kStarted);
 }
 
 void DisplaySourceCustomBindings::OnSessionTerminated(int sink_id) {
@@ -250,6 +281,7 @@ void DisplaySourceCustomBindings::OnSessionTerminated(int sink_id) {
   CHECK(session);
   session_map_.erase(sink_id);
   DispatchSessionTerminated(sink_id);
+  CallCompletionCallback(sink_id, kTerminated);
 }
 
 void DisplaySourceCustomBindings::OnSessionError(int sink_id,
@@ -257,14 +289,15 @@ void DisplaySourceCustomBindings::OnSessionError(int sink_id,
                                                  const std::string& message) {
   DisplaySourceSession* session = GetDisplaySession(sink_id);
   CHECK(session);
-  if (session->state() != DisplaySourceSession::Established &&
-      session->state() != DisplaySourceSession::Terminating) {
-    // Error has occured before the session has actually started,
-    // no need to wait for session termination notification.
-    session_map_.erase(sink_id);
-  }
-
   DispatchSessionError(sink_id, type, message);
 }
 
+DisplaySourceCustomBindings::CallbackInfo
+DisplaySourceCustomBindings::GetCallbackInfo(
+    DisplaySourceCustomBindings::CallbackType type,
+    int sink_id) const {
+  static int sCallId = 0;
+  return {type, sink_id, ++sCallId};
+}
+
 }  // extensions
diff --git a/extensions/renderer/display_source_custom_bindings.h b/extensions/renderer/display_source_custom_bindings.h
index 199f199..2a65445 100644
--- a/extensions/renderer/display_source_custom_bindings.h
+++ b/extensions/renderer/display_source_custom_bindings.h
@@ -29,8 +29,12 @@ class DisplaySourceCustomBindings : public ObjectBackedNativeHandler {
       const v8::FunctionCallbackInfo<v8::Value>& args);
   void TerminateSession(
       const v8::FunctionCallbackInfo<v8::Value>& args);
-
-  void DispatchSessionStarted(int sink_id) const;
+  // Call completion callbacks.
+  enum CallbackType { kStarted, kTerminated };
+  void CallCompletionCallback(int sink_id,
+                              CallbackType type,
+                              const std::string& error_message = "");
+  // Dispatch events
   void DispatchSessionTerminated(int sink_id) const;
   void DispatchSessionError(int sink_id,
                             DisplaySourceErrorType type,
@@ -46,6 +50,16 @@ class DisplaySourceCustomBindings : public ObjectBackedNativeHandler {
   DisplaySourceSession* GetDisplaySession(int sink_id) const;
 
   std::map<int, scoped_ptr<DisplaySourceSession>> session_map_;
+  // Data of a call completion callback.
+  struct CallbackInfo {
+    CallbackType type;
+    int sink_id;
+    int call_id;  // Each call has a unique Id.
+  };
+
+  CallbackInfo GetCallbackInfo(CallbackType type, int sink_id) const;
+
+  std::vector<CallbackInfo> callbacks_;
   base::WeakPtrFactory<DisplaySourceCustomBindings> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DisplaySourceCustomBindings);
diff --git a/extensions/renderer/resources/display_source_custom_bindings.js b/extensions/renderer/resources/display_source_custom_bindings.js
index 13e8167..a1345ff 100644
--- a/extensions/renderer/resources/display_source_custom_bindings.js
+++ b/extensions/renderer/resources/display_source_custom_bindings.js
@@ -8,34 +8,62 @@ var binding = require('binding').Binding.create('displaySource');
 var chrome = requireNative('chrome').GetChrome();
 var lastError = require('lastError');
 var natives = requireNative('display_source');
+var logging = requireNative('logging');
+
+var callbacksInfo = {};
+
+function callbackWrapper(callback, method, message) {
+  if (callback == undefined)
+    return;
+
+  try {
+    if (message !== null)
+      lastError.set('displaySource.startSession', message, null, chrome);
+    callback();
+  } finally {
+    lastError.clear(chrome);
+  }
+}
+
+function callCompletionCallback(callbackId, error_message) {
+  try {
+    var callbackInfo = callbacksInfo[callbackId];
+    logging.DCHECK(callbackInfo != null);
+    callbackWrapper(callbackInfo.callback, callbackInfo.method, error_message);
+  } finally {
+    delete callbacksInfo[callbackId];
+  }
+}
 
 binding.registerCustomHook(function(bindingsAPI, extensionId) {
   var apiFunctions = bindingsAPI.apiFunctions;
-  apiFunctions.setHandleRequest('startSession',
-      function(sessionInfo, callback) {
+  apiFunctions.setHandleRequest(
+      'startSession', function(sessionInfo, callback) {
         try {
-          natives.StartSession(sessionInfo);
+          var callId = natives.StartSession(sessionInfo, callbackWrapper);
+          callbacksInfo[callId] = {
+            callback: callback,
+            method: 'displaySource.startSession'
+          };
         } catch (e) {
-          lastError.set('displaySource.startSession', e.message, null, chrome);
-        } finally {
-           if (callback !== undefined)
-             callback();
-           lastError.clear(chrome);
+          callbackWrapper(callback, 'displaySource.startSession', e.message);
         }
-  });
-  apiFunctions.setHandleRequest('terminateSession',
-      function(sink_id, callback) {
+      });
+  apiFunctions.setHandleRequest(
+      'terminateSession', function(sink_id, callback) {
         try {
-          natives.TerminateSession(sink_id);
+          var callId = natives.TerminateSession(sink_id, callbackWrapper);
+          callbacksInfo[callId] = {
+            callback: callback,
+            method: 'displaySource.terminateSession'
+          };
         } catch (e) {
-          lastError.set(
-              'displaySource.terminateSession', e.message, null, chrome);
-        } finally {
-           if (callback !== undefined)
-             callback();
-           lastError.clear(chrome);
+          callbackWrapper(
+              callback, 'displaySource.terminateSession', e.message);
         }
-  });
+      });
 });
 
 exports.$set('binding', binding.generate());
+// Called by C++.
+exports.$set('callCompletionCallback', callCompletionCallback);