Update client side navigator.connect API to use ServicePortCollection [1/3]

Source/modules/navigatorconnect/ServicePort.cpp

+// Copyright 2015 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 "config.h"
+#include "modules/navigatorconnect/ServicePort.h"
+
+#include "bindings/core/v8/ScriptValue.h"
+#include "bindings/core/v8/SerializedScriptValueFactory.h"
+#include "core/dom/MessagePort.h"
+#include "modules/navigatorconnect/ServicePortCollection.h"
+#include "public/platform/Platform.h"
+#include "public/platform/modules/navigator_services/WebServicePortProvider.h"
+
+namespace blink {
+
+ServicePort* ServicePort::create(WeakPtrWillBeRawPtr<ServicePortCollection> collection, const WebServicePort& port)
+{
+    return new ServicePort(collection, port);
+}
+
+ServicePort::~ServicePort()
+{
+    if (m_isOpen && m_collection) {
+        m_collection->closePort(this);
+    }
+}
+
+String ServicePort::targetURL() const
+{
+    return m_port.targetUrl.string();
+}
+
+String ServicePort::name() const
+{
+    return m_port.name;
+}
+
+ScriptValue ServicePort::data(ScriptState* scriptState) const
+{
+    if (!m_serializedData)
+        return ScriptValue::createNull(scriptState);
+
+    return ScriptValue(scriptState, m_serializedData->deserialize(scriptState->isolate()));
+}
+
+void ServicePort::postMessage(ExecutionContext* executionContext, PassRefPtr<SerializedScriptValue> message, const MessagePortArray* ports, ExceptionState& exceptionState)
+{
+    OwnPtr<MessagePortChannelArray> channels;
+    if (ports) {
+        channels = MessagePort::disentanglePorts(executionContext, ports, exceptionState);
+        if (exceptionState.hadException())
+            return;
+    }
+
+    WebString messageString = message->toWireString();
+    OwnPtr<WebMessagePortChannelArray> webChannels = MessagePort::toWebMessagePortChannelArray(channels.release());
+    if (m_collection) {
+        WebServicePortProvider* provider = m_collection->provider();
+        provider->postMessage(m_port.id, messageString, webChannels.leakPtr());
+    }
+}
+
+void ServicePort::close()
+{
+    // TODO(mek): Figure out if this should throw instead of just quietly fail.
+    if (!m_isOpen)
+        return;
+
+    if (m_collection) {
+        m_collection->closePort(this);
+        m_collection = nullptr;
+    }
+    m_isOpen = false;
+}
+
+ServicePort::ServicePort(WeakPtrWillBeRawPtr<ServicePortCollection> collection, const WebServicePort& port)
+    : m_isOpen(true), m_port(port), m_collection(collection)
+{
+    if (!m_port.data.isEmpty()) {
+        m_serializedData = SerializedScriptValueFactory::instance().createFromWire(m_port.data);

[scheib, 2015/06/24 04:16:55]

Just checking, but there can be different contexts when the data is later
deserialized? Just curious why it's deserialized each time it is accessed from
the attribute. 

[Marijn Kruisselbrink, 2015/06/24 17:03:27]

Great question, I should probably add a comment, although I'm not entirely sure
myself. To start with, this is the same thing Notification.data does
(https://codereview.chromium.org/993893002), although that is a terrible reason
without understanding why that code does it that way of course. My limited
understanding of bindings, v8 etc is that this possibly could indeed be called
from multiple contexts. As I understand it (but my understanding could be
completely wrong) the same ServicePort instance can get accessed from multiple
isolated worlds, and this should return a ScriptValue that was deserialized in
the world it is returned to (partly because SerializedScriptValueFactory uses
ScriptState::current() to deserialize the message, and deserializing can
potentially have side effects/cause arbitrary code to be executed in that
context).

This could probably use caching similar to how MessageEvent caches its
deserialized script values of course, I'll add a TODO for that.

[haraken, 2015/06/24 23:50:31]

Yes, your understanding is correct. Given that we must not leak ScriptValues
between isolated worlds, we must serialize/deserialize the value. Basically a
C++ DOM object must not hold a ScriptValue (unless you're understanding what
you're doing :-) because the C++ DOM object is shared between multiple isolated
worlds.

Another reason is that ScriptValue holds a strong Persistent reference from
Blink to V8. This has a risk of producing leak (this "leak" means "memory
leak"). SerializedScriptValue doesn't have a risk.

+    }
+}
+
+} // namespace blink
+