[Presentation API] (4th)(1-UA blink side) Add WebPresentationConnection and WebPresentationConnecti…

content/renderer/presentation/presentation_connection_proxy.cc

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/presentation/presentation_connection_proxy.h"
+
+#include "base/logging.h"
+#include "third_party/WebKit/public/platform/WebString.h"
+#include "third_party/WebKit/public/platform/modules/presentation/WebPresentationConnection.h"
+#include "third_party/WebKit/public/platform/modules/presentation/WebPresentationConnectionClient.h"
+
+namespace content {
+
+PresentationConnectionProxy::PresentationConnectionProxy()
+    : binding_(this),
+      target_connection_(nullptr),
+      source_connection_(nullptr) {}

[mark a. foltz, 2016/12/02 22:44:00]

If the proxy can make these values mandatory in the ctor, it would make the
class more robust: it looks like little or none of the API will work without
these set.

[zhaobin, 2016/12/05 20:28:16]

Pass in on_message_callback.

For source_connection_, Proxy holds reference to PresentationConnection, and
PresentationConnection holds reference to Proxy as well. It seems that we need
to 1. create Proxy; 2. create PresentationConnection with Proxy; 3 set
proxy.source_connection_ in PresentationConnection ctor. (Or it will become 1.
create PresentationConnection; 2. create Proxy with PresentationConnection; 3.
set PresentationConnection.m_proxy in Proxy ctor. Will it be similar situation,
since we cannot garantee validity of PresentationConnection.m_proxy?)

target_connection_ (receiver connection proxy) for controller is not available
when creating controller connection proxy. Will have to set it later.

[mark a. foltz, 2016/12/05 22:07:29]

I slightly prefer the latter approach, since you can control the exposure of the
proxy via the PresentationConnection and guarantee the proxy has a source
connection set.

+
+PresentationConnectionProxy::~PresentationConnectionProxy() = default;
+
+blink::mojom::PresentationConnectionPtr PresentationConnectionProxy::Bind() {
+  return binding_.CreateInterfacePtrAndBind();
+}
+
+void PresentationConnectionProxy::SendSessionMessage(
+    blink::mojom::PresentationSessionInfoPtr session,
+    blink::mojom::SessionMessagePtr session_message) const {
+  DCHECK(target_connection_);
+  target_connection_->OnMessage(std::move(session_message),
+                                on_message_callback_);
+}
+
+void PresentationConnectionProxy::SetSourceConnection(
+    blink::WebPresentationConnection* connection) {
+  DCHECK(!source_connection_);
+  source_connection_ = connection;
+}
+
+void PresentationConnectionProxy::SetTargetConnection(
+    blink::mojom::PresentationConnectionPtr connection) {
+  DCHECK(!target_connection_);
+  target_connection_ = std::move(connection);
+  target_connection_->DidChangeState(
+      blink::mojom::PresentationConnectionState::CONNECTED);
+}
+
+void PresentationConnectionProxy::OnMessage(
+    blink::mojom::SessionMessagePtr session_message,
+    const OnMessageCallback& callback) {
+  DCHECK(source_connection_);

[mark a. foltz, 2016/12/02 22:44:00]

What guarantees that |source_connection_| is set before OnMessage is invoked?

[zhaobin, 2016/12/05 20:28:16]

(Sorry for the confusion, all invocation code are in patch 5) We sets
source_connection_ in PresentationConnection ctor (which will be right after
PresentationConnectionProxy ctor due to that cross reference issue...). When
this method is called, we should already have a PresentationConnection object,
and a valid source_connection_.

+  DCHECK(!callback.is_null());

[mark a. foltz, 2016/12/02 22:44:00]

What guarantees that |on_message_callback_| is set before OnMessage is invoked?

[zhaobin, 2016/12/05 20:28:16]

Done.

+
+  switch (session_message->type) {
+    case blink::mojom::PresentationMessageType::TEXT: {
+      DCHECK(session_message->message);
+      source_connection_->didReceiveTextMessage(
+          blink::WebString::fromUTF8(session_message->message.value()));
+      break;
+    }
+    case blink::mojom::PresentationMessageType::ARRAY_BUFFER:
+    case blink::mojom::PresentationMessageType::BLOB: {
+      DCHECK(session_message->data);
+      source_connection_->didReceiveBinaryMessage(
+          &(session_message->data->front()), session_message->data->size());
+      break;
+    }
+    default: {
+      NOTREACHED();

[mark a. foltz, 2016/12/02 22:44:00]

callback.Run(false) here?

[zhaobin, 2016/12/05 20:28:17]

Done.

+      break;
+    }
+  }
+
+  callback.Run(true);

[mark a. foltz, 2016/12/02 22:44:00]

Unless you pass false to the callback somewhere, the callback doesn't need to
take a parameter.

[zhaobin, 2016/12/05 20:28:16]

Done.

+}
+
+void PresentationConnectionProxy::DidChangeState(
+    blink::mojom::PresentationConnectionState state) {
+  DCHECK(source_connection_);

[mark a. foltz, 2016/12/02 22:44:00]

What guarantees that |source_connection_| is set when DidChangeState is invoked?

[zhaobin, 2016/12/05 20:28:16]

Same as above.

+  if (state == blink::mojom::PresentationConnectionState::CONNECTED) {
+    source_connection_->didChangeState(
+        blink::WebPresentationConnectionState::Connected);
+  } else {
+    // |DidChangeState| should only handles state transition from connecting ->
+    // connected. PresentationService and MRP handles other state transitions.
+    NOTREACHED();
+  }
+}
+
+void PresentationConnectionProxy::RegisterOnMessageCallback(
+    const OnMessageCallback& callback) {
+  on_message_callback_ = callback;
+}
+
+}  // namespace content