Adds NetInfo v3 Web API to Blink.

Source/modules/netinfo/NetworkInfoConnection.cpp

+// Copyright 2014 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/netinfo/NetworkInfoConnection.h"
+
+#include "RuntimeEnabledFeatures.h"
+#include "core/dom/ExecutionContext.h"
+#include "core/page/NetworkStateNotifier.h"
+#include "core/workers/WorkerGlobalScope.h"

[adamk, 2014/05/27 21:44:27]

Hmm, do you need this include for something?

[jkarlin, 2014/05/28 13:31:30]

Done.

+#include "modules/EventTargetModules.h"
+#include "public/platform/WebString.h"

[adamk, 2014/05/27 21:44:27]

I don't think you want this include

[jkarlin, 2014/05/28 13:31:30]

Done.

+#include "v8.h"

[adamk, 2014/05/27 21:44:27]

Or this one

[jkarlin, 2014/05/28 13:31:30]

Done.

+
+namespace {
+
+const char kCellular[] = "cellular";
+const char kBluetooth[] = "bluetooth";
+const char kEthernet[] = "ethernet";
+const char kWifi[] = "wifi";
+const char kOther[] = "other";
+const char kNone[] = "none";
+
+String connectionTypeToString(blink::WebConnectionType type)
+{
+    switch (type) {
+    case blink::ConnectionTypeCellular:
+        return kCellular;
+    case blink::ConnectionTypeBluetooth:
+        return kBluetooth;
+    case blink::ConnectionTypeEthernet:
+        return kEthernet;
+    case blink::ConnectionTypeWifi:
+        return kWifi;
+    case blink::ConnectionTypeOther:
+        return kOther;
+    case blink::ConnectionTypeNone:
+        return kNone;
+    }
+    ASSERT_NOT_REACHED();
+    return kNone;
+}
+
+} // namespace
+
+namespace WebCore {
+
+PassRefPtrWillBeRawPtr<NetworkInfoConnection> NetworkInfoConnection::create(ExecutionContext* context)
+{
+    RefPtrWillBeRawPtr<NetworkInfoConnection> connection(adoptRefWillBeRefCountedGarbageCollected(new NetworkInfoConnection(context)));
+    connection->suspendIfNeeded();
+    return connection.release();
+}
+
+NetworkInfoConnection::~NetworkInfoConnection()
+{
+    ASSERT(!m_observing);
+}
+
+String NetworkInfoConnection::type() const
+{
+    return connectionTypeToString(m_type);
+}
+
+void NetworkInfoConnection::connectionTypeChange(blink::WebConnectionType type)
+{
+    ASSERT(executionContext()->isContextThread());
+
+    if (m_type == type)

[adamk, 2014/05/27 21:44:27]

Doesn't NetworkChangeNotifier guarantee that it'll only notify you if type
actually changes? That is, can this be an ASSERT?

[jkarlin, 2014/05/28 13:31:30]

It's actually possible for the same observer to get called twice for a single
event if it removes and then adds itself again during notification. Added a
comment here and to NetworkStateNotifier.

+        return;
+
+    m_type = type;
+    dispatchEvent(Event::create(EventTypeNames::typechange));
+}
+
+const AtomicString& NetworkInfoConnection::interfaceName() const
+{
+    return EventTargetNames::NetworkInfoConnection;
+}
+
+ExecutionContext* NetworkInfoConnection::executionContext() const
+{
+    return ActiveDOMObject::executionContext();
+}
+
+bool NetworkInfoConnection::addEventListener(const AtomicString& eventType, PassRefPtr<EventListener> listener, bool useCapture)
+{
+    bool hadEvents = hasEventListeners();

[adamk, 2014/05/27 21:44:27]

This call to hasEventListeners() seems unnecessary, since you already have
m_observing guards in startObserving().

[jkarlin, 2014/05/28 13:31:30]

Done.

+    bool ret = EventTargetWithInlineData::addEventListener(eventType, listener, useCapture);

[adamk, 2014/05/27 21:44:27]

The return value should be telling you whether the event listener got added, so
this could be restructured as:

if (!EventTargetWithInlineData::addEventListener(eventType, listener,
useCapture))
    return false;
startObserving();
return true;

Hmm, that may or may not be worth it. It just looks funny to me to treat the
bool return value as opaque even though you actually care about that signal.

[jkarlin, 2014/05/28 13:31:30]

Done.

+    if (!hadEvents && hasEventListeners())
+        startObserving();
+    return ret;
+}
+
+bool NetworkInfoConnection::removeEventListener(const AtomicString& eventType, EventListener* listener, bool useCapture)
+{
+    bool hadEvents = hasEventListeners();

[adamk, 2014/05/27 21:44:27]

Same here, no need to store the previous value.

[jkarlin, 2014/05/28 13:31:30]

Done.

+    bool ret = EventTargetWithInlineData::removeEventListener(eventType, listener, useCapture);

[adamk, 2014/05/27 21:44:27]

Same here, this could be:

if (!EventTargetWithInlineData::removeEventListener(eventType, listener,
useCapture))
    return false;
if (!hasEventListeners())
    stopObserving();
return true;

Again, not totally clear to me that this is better, but it at least makes use of
the return value.

[jkarlin, 2014/05/28 13:31:30]

Done.

+    if (hadEvents && !hasEventListeners())
+        stopObserving();
+    return ret;
+}
+
+void NetworkInfoConnection::removeAllEventListeners()
+{
+    bool hadEvents = hasEventListeners();

[adamk, 2014/05/27 21:44:27]

And here.

[jkarlin, 2014/05/28 13:31:30]

Done.

+    EventTargetWithInlineData::removeAllEventListeners();
+    if (hadEvents && !hasEventListeners())

[adamk, 2014/05/27 21:44:27]

I'd rather that you change this to an ASSERT(!hasEventListeners())...

[jkarlin, 2014/05/28 13:31:30]

Done.

+        stopObserving();

[adamk, 2014/05/27 21:44:27]

...and then always call stopObserving() when removeAllEventListeners() is
called.

[jkarlin, 2014/05/28 13:31:30]

Done.

+}
+
+void NetworkInfoConnection::trace(Visitor*)
+{
+}
+
+bool NetworkInfoConnection::hasPendingActivity() const
+{
+    // Prevent collection of this object when there are active listeners.

[adamk, 2014/05/27 21:44:27]

Can you add an ASSERT(hasEventListeners()) here?

[jkarlin, 2014/05/28 13:31:30]

Done.

+    return m_observing;
+}
+
+void NetworkInfoConnection::stop()
+{
+    stopObserving();
+}
+
+void NetworkInfoConnection::startObserving()
+{
+    if (!m_observing) {
+        networkStateNotifier().addObserver(this, executionContext());
+        m_observing = true;
+    }
+}
+
+void NetworkInfoConnection::stopObserving()
+{
+    if (m_observing) {
+        networkStateNotifier().removeObserver(this, executionContext());
+        m_observing = false;
+    }
+}
+
+NetworkInfoConnection::NetworkInfoConnection(ExecutionContext* context)
+    : ActiveDOMObject(context)
+    , m_observing(false)
+{
+    ScriptWrappable::init(this);
+    m_type = networkStateNotifier().connectionType();
+}
+
+} // namespace WebCore