Implement BroadcastChannel

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

+// 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.
+
+#ifndef BroadcastChannelConnection_h
+#define BroadcastChannelConnection_h
+
+#include "components/webmessaging/public/interfaces/broadcast_channel.mojom-blink.h"
+#include "mojo/public/cpp/bindings/strong_associated_binding.h"
+#include "platform/weborigin/SecurityOriginHash.h"
+#include "wtf/ListHashSet.h"
+#include "wtf/text/StringHash.h"
+
+namespace blink {
+
+// To ensure proper message ordering, all BroadcastChannel instances on the same
+// thread share a single mojo connection. This class represents such a
+// connection. BroadcastChannel instances register themselves with the correct
+// connection, and use the connection to broadcast messages.
+class BroadcastChannelConnection final : public webmessaging::mojom::blink::BroadcastChannelClient {
+    WTF_MAKE_NONCOPYABLE(BroadcastChannelConnection);
+public:
+    class Client {

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

We should make this a GarbageCollectedMixin. Currently
BroadcastChannelConnection::m_clients holds raw pointers to on-heap objects,
which is potentially unsafe.

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

Unfortunately I couldn't figure out a way to do what I needed to do without
holding raw pointers to on-heap objects... So the whole point of having this
Client interface rather than just storing BroadcastChannel references directly
was to get around the compiler complaints about storing pointers to on-heap
objects. See my other answers for more explanation of the what and why.

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

This Client must be GarbageCollectedMixin, because it is inherited by a
GarbageCollected object.

+    public:
+        virtual String name() const = 0;
+        virtual SecurityOrigin* origin() const = 0;
+
+        // Called when a message is received for this channel. Not called for
+        // messages send by this client to the channel.
+        virtual void onMessage(const String& message) = 0;
+
+        // Called when the connection this client is registered with closes.
+        // After onError is called a client should not try to do anything with
+        // the connection anymore.
+        virtual void onError() = 0;
+    };
+
+    // Returns the BroadcastChannelConnection instance to use for the channel
+    // identified by the particular origin and name. When called with the same
+    // parameters on the same thread this will always return the same instance,
+    // except if for some reason the mojo connection got severed.
+    // If all clients using this particular connection get closed, the
+    // connection itself might also be closed, in which case the next time the
+    // method is called a new connection is created.
+    static BroadcastChannelConnection* getForChannel(const RefPtr<SecurityOrigin>&, const String& name);
+    ~BroadcastChannelConnection();
+
+    // Register a new client that uses this connection. The client must call
+    // unregisterClient when the client object is destroyed.
+    void registerClient(Client*);
+    void unregisterClient(Client*);
+
+    // Broadcasts a message to the channel this connection is for. The message
+    // will be delivered to all clients except the client that send the message.
+    void broadcast(Client*, const String& message);
+private:
+    BroadcastChannelConnection(const String& name, const RefPtr<SecurityOrigin>&, webmessaging::mojom::blink::BroadcastChannelClientAssociatedPtrInfo*, mojo::AssociatedGroup*);
+
+    // webmessaging::mojom::blink::BroadcastChannelClient:
+    void OnMessage(const String& message) override;
+
+    String m_name;
+    RefPtr<SecurityOrigin> m_origin;
+    ListHashSet<Client*> m_clients;

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

Does it need to be a ListHashSet? I guess HashSet would be enough. (Note: The
implementation of ListHashSet is complicated and heavy.)

I think you should use HeapHashSet<WeakMember>. Then you don't need to call
unregister() because the unregistration is automatically done by weak
processing.

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

The List parts of ListHashSet (specifically the ordered by insertion order part)
is in fact a hard requirement. The spec requires dispatching events to
BroadcastChannel instances (when those instances live on the same event loop) to
be done in creation order of the BroadcastChannel instances. So I need some kind
of container that maintains insertion order to store this list. I don't care
particularly about the HashSet part, since it's unlikely that there will be
enough BroadcastChannel instances for any particular channel in any particular
thread for the performance difference when removing channels to be relevant, but
since it exists I might as well use it.
Even with some hypothetical OrderedContainer<WeakMember<...>>, if there was any
kind of container that maintained ordering and would support weak members, I'd
still need some kind of unregister() (or a way to tell the container to call
some method of me when the container empties itself), as I want to be able to
close the mojo connection when no more BroadcastChannel instances remain for
this particular channel.

Maybe that could be made to work differently by changing the way ownership of
BroadcastChannelConnection works (see my replyto your questions about the
connection map stuff), but I'm not sure it would be any clearer, and certainly
non-obvious that it would be correct.

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

You can use HeapListHashSet<WeakMember<>>.

Also you can keep the unregister method. My point is that you must not call the
unregister method in the destructor of Client. A destructor is not allowed to
touch another on-heap object (because the destruction order is undterministic).
That's why you need to use weak members and ask the Oilpan to unregister the
Client from the hash set.

+    mojo::StrongAssociatedBinding<webmessaging::mojom::blink::BroadcastChannelClient> m_binding;
+};
+
+} // namespace blink
+
+#endif // BroadcastChannelConnection_h