Implement BroadcastChannel

third_party/WebKit/Source/modules/broadcastchannel/BroadcastChannelConnection.cpp

+// 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 "modules/broadcastchannel/BroadcastChannelConnection.h"
+
+#include "platform/ThreadSafeFunctional.h"
+#include "platform/mojo/MojoHelper.h"
+#include "public/platform/Platform.h"
+#include "public/platform/ServiceRegistry.h"
+#include "public/platform/WebTaskRunner.h"
+#include "public/platform/WebTraceLocation.h"
+
+namespace blink {
+
+namespace {
+
+void connectToService(webmessaging::mojom::blink::BroadcastChannelServiceRequest request)
+{
+    DCHECK(Platform::current()->mainThread()->isCurrentThread());
+    Platform::current()->serviceRegistry()->connectToRemoteService(std::move(request));
+}
+
+webmessaging::mojom::blink::BroadcastChannelServicePtr& getThreadSpecificService()
+{
+    DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<webmessaging::mojom::blink::BroadcastChannelServicePtr>, service, new ThreadSpecific<webmessaging::mojom::blink::BroadcastChannelServicePtr>);
+    if (!service.isSet()) {
+        Platform::current()->mainThread()->getWebTaskRunner()->postTask(BLINK_FROM_HERE,
+            threadSafeBind(&connectToService, passed(mojo::GetProxy(&*service))));
+    }
+    return *service;
+}
+
+using ConnectionKey = std::pair<RefPtr<SecurityOrigin>, String>;
+using ConnectionMap = HashMap<ConnectionKey, BroadcastChannelConnection*>;
+
+ConnectionMap* getConnectionMap()
+{
+    DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<ConnectionMap>, connectionMap, new ThreadSpecific<ConnectionMap>);

[haraken, 2016/06/02 04:23:41]

It's a bit nasty to use thread-local storage for this. Isn't there any object
that should own the connection map?

[Marijn Kruisselbrink, 2016/06/02 18:33:29]

All BroadcastChannel instances that share thread, name and origin need to
somehow coordinate with eachother before hitting IPC. In an earlier patch (up
till PS4 I believe) I did that by having a more complicated mojo API (having
multiple layers of associated interfaces to ensure all messages related to all
broadcast channels from a particular thread are send and received in the correct
order), which also had the downside that go to/from the same thread still had to
go over IPC multiple times.

So the slightly more complicated design of explicitly keeping track of all
BroadcastChannel instances on the renderer side (on a per thread, per
origin/name basis). Rather than per-thread this could be per-process, but that
would make the code even more complicated.

So while the connections themselves could be jointly "owned" by the channel
instances that use the connection, new channel instances will still need a way
to find a potentially existing connection to reuse. 

+    return connectionMap;
+}
+
+} // namespace
+
+// static
+BroadcastChannelConnection* BroadcastChannelConnection::getForChannel(const RefPtr<SecurityOrigin>& origin, const String& name)
+{
+    ConnectionMap* connectionMap = getConnectionMap();
+    auto it = connectionMap->find(ConnectionKey(origin, name));
+    if (it != connectionMap->end())
+        return it->value;
+
+    webmessaging::mojom::blink::BroadcastChannelServicePtr& service = getThreadSpecificService();
+    webmessaging::mojom::blink::BroadcastChannelClientAssociatedPtrInfo localClientInfo;
+    BroadcastChannelConnection* connection = new BroadcastChannelConnection(name, origin, &localClientInfo, service.associated_group());
+    service->Subscribe(origin, name, std::move(localClientInfo));
+    return connection;
+}
+
+BroadcastChannelConnection::~BroadcastChannelConnection()
+{
+    getConnectionMap()->remove(ConnectionKey(m_origin, m_name));
+    for (Client* client : m_clients)
+        client->onError();
+}
+
+void BroadcastChannelConnection::registerClient(Client* client)
+{
+    m_clients.add(client);
+}
+
+void BroadcastChannelConnection::unregisterClient(Client* client)
+{
+    m_clients.remove(client);
+    if (m_clients.isEmpty())
+        delete this;

[haraken, 2016/06/02 04:23:41]

Can we use a unique_ptr to avoid calling manual delete? Blink doesn't prefer
using manual delete.

[Marijn Kruisselbrink, 2016/06/02 18:33:29]

The lifetime of this class is mostly managed by the
mojo::StrongAssociatedBinding (which tied the lifetime of an implementation of a
mojo interface to the lifetime of the underlying mojo connection). But
additionally we also want to close the connection explicitly when all clients
for a particular connection go away (to not leak memory/connections). Hence the
delete this.

I could change this ownership by making the ConnectionMap store unique_ptr<>. I
would then have to go back to using mojo::AssociatedBinding (which doesn't
manage lifetime), and add my own connection error handler that removes the
conenction from the map (thereby destroying the connection instance). And the
delete this here would then also change to remove the connection from the map.

A downside of that would be that at least with "delete this" it is obvious that
this is being deleted. If the code here just removes this from a map it isn't
clear at all that the result of that is |this| being invalid afterwards. Also
it's more code since I'll need my own mojo error handlers. So it's unclear to me
what the benefit would be.

What maybe might work is to make BroadcastChannelConnection garbage collected
(or refcounted), and have the BroadcastChannel instances that together want to
keep it alive make sure of that by having a (strong) reference to the
connection. As mentioned in one of my other comments I would still need to
somehow have an ordered list of weak references to the BroadcastChannel
instances from the Connection though. And on top of that if
BroadcastChannelConnection would be garbage collected, the ConnectionMap becomes
another problem of wanting to store weak references to garbage collected objects
in a container that doesn't currently support weak references (as new
BroadcastChannel instances would still need to be able to somehow find the
correct Connection to use, thus the need for some kind of (origin, name) =>
connection map).

So it seems that doing that would maybe make some things work more
automagically, but would also require more hacks in other places. So with that
in mind it seemed much preferable to have simpler code that is easier to reason
about (since all the memory management logic is fairly self contained), rather
than having code that still requires some manual bookkeeping but is much harder
to reason about since now the bookkeeping interacts with garbage collection in
interesting ways...

[haraken, 2016/06/03 05:33:03]

We normally use a dispose() method to promptly dispose something (before waiting
for a destructor). You can explicitly call dispose() here and let a GC destruct
the object later.

+}
+
+void BroadcastChannelConnection::broadcast(Client* from, const String& message)
+{
+    for (Client* client : m_clients) {
+        if (client != from)
+            client->onMessage(message);
+    }
+    getThreadSpecificService()->Broadcast(from->origin(), from->name(), message);
+}
+
+BroadcastChannelConnection::BroadcastChannelConnection(const String& name, const RefPtr<SecurityOrigin>& origin, webmessaging::mojom::blink::BroadcastChannelClientAssociatedPtrInfo* clientInfo, mojo::AssociatedGroup* associatedGroup)
+    : m_name(name)
+    , m_origin(origin)
+    , m_binding(this, clientInfo, associatedGroup)
+{
+    auto result = getConnectionMap()->set(ConnectionKey(m_origin, m_name), this);
+    DCHECK(result.isNewEntry) << "There should only be one connection per channel per thread.";
+    ALLOW_UNUSED_LOCAL(result);
+}
+
+void BroadcastChannelConnection::OnMessage(const String& message)
+{
+    for (Client* client : m_clients)
+        client->onMessage(message);
+}
+
+} // namespace blink