Adds type information to the NetworkStateNotifier. Also allows for registration of observers.

Source/core/page/NetworkStateNotifier.cpp

 /*
  * Copyright (C) 2008 Apple Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
  *    documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
  * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
  * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
  * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "core/page/NetworkStateNotifier.h"
 
+#include "core/dom/ExecutionContext.h"
 #include "core/page/Page.h"
 #include "wtf/Assertions.h"
+#include "wtf/Functional.h"
 #include "wtf/MainThread.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/Threading.h"
 
 namespace WebCore {
 
+NetworkStateNotifier::~NetworkStateNotifier()
+{
+    deleteAllValues(m_observers);

[adamk, 2014/05/21 09:21:31]

This won't be needed if you use OwnPtrs as values.

[jkarlin, 2014/05/21 14:23:17]

Done.

+}
+
 NetworkStateNotifier& networkStateNotifier()
 {
     AtomicallyInitializedStatic(NetworkStateNotifier*, networkStateNotifier = new NetworkStateNotifier);
     return *networkStateNotifier;
 }
 
 void NetworkStateNotifier::setOnLine(bool onLine)
 {
     ASSERT(isMainThread());
 
     {
         MutexLocker locker(m_mutex);
         if (m_isOnLine == onLine)
             return;
 
         m_isOnLine = onLine;
     }
 
     Page::networkStateChanged(onLine);
 }
 
+void NetworkStateNotifier::setWebConnectionType(blink::WebNetworkConnection::ConnectionType type)
+{
+    ASSERT(isMainThread());
+
+    MutexLocker locker(m_mutex);
+    if (m_type == type)
+        return;
+    m_type = type;
+
+    for (ObserverListMap::iterator it = m_observers.begin(); it != m_observers.end(); ++it) {
+        ExecutionContext* context = it->key;
+        context->postTask(bind(&NetworkStateNotifier::notifyObserversOnContext, this, context, type));
+    }
 }
+
+void NetworkStateNotifier::addObserver(NetworkStateObserver* observer, ExecutionContext* context)
+{
+    ASSERT(context->isContextThread());
+    ASSERT(observer);
+
+    MutexLocker locker(m_mutex);
+    ObserverListMap::iterator it = m_observers.find(context);
+    if (it == m_observers.end()) {
+        m_observers.set(context, new ObserverList());
+        it = m_observers.find(context);

[adamk, 2014/05/21 09:21:31]

The idiomatic way to handle this song and dance in a WTF HashMap is:

ObserverListMap::AddResult result = m_observers.add(context, nullptr);
if (result.isNewEntry)
  result.storedValue->value = adoptPtr(new ObserverList);
result.storedValue->value->observers.append(observer);

(which does only a single hash lookup instead of the 3 here; I realize
addObserver is probably not performance sensitive, but being idiomatic is easier
to read for me)

[jkarlin, 2014/05/21 14:23:17]

Done.

+    }
+
+    it->value->observers.append(observer);

[adamk, 2014/05/21 09:21:31]

Maybe add an ASSERT that the observer isn't already in the list.

[jkarlin, 2014/05/21 14:23:17]

Done.

+}
+
+void NetworkStateNotifier::removeObserver(NetworkStateObserver* observer, ExecutionContext* context)
+{
+    ASSERT(context->isContextThread());
+    ASSERT(observer);
+
+    ObserverList* observerList = lockAndFindObserverList(context);
+    if (!observerList)
+        return;
+
+    Vector<NetworkStateObserver*>& observers = observerList->observers;
+    size_t index = observers.find(observer);
+    if (index != kNotFound) {
+        observers[index] = 0;
+        observerList->zeroedObservers.append(index);
+    }
+
+    if (!observerList->iterating && !observerList->zeroedObservers.isEmpty())
+        collectZeroedObservers(context);

[adamk, 2014/05/21 09:21:31]

All this zeroed observers logic is pretty tricky. Is there a way to write
unittests for it?

[jkarlin, 2014/05/21 14:23:17]

Done.

+}
+
+void NetworkStateNotifier::notifyObserversOnContext(ExecutionContext* context, blink::WebNetworkConnection::ConnectionType type)
+{
+    ASSERT(context->isContextThread());
+
+    ObserverList* observerList = lockAndFindObserverList(context);
+
+    // The context could have been removed before the notification task got to run.

[adamk, 2014/05/21 09:21:31]

If this happens then the ASSERT above could crash. In general, I think you have
to first do a lookup in m_observers before accessing context (unless it's passed
in addObserver or removeObserver, naturally).

[jkarlin, 2014/05/21 14:23:17]

Done.

+    if (!observerList)
+        return;
+
+    observerList->iterating = true;
+
+    for (size_t i = 0; i < observerList->observers.size(); ++i) {
+        // Observers removed during iteration are zeroed out, skip them.
+        if (observerList->observers[i])
+            observerList->observers[i]->connectionTypeChange(type);
+    }
+
+    observerList->iterating = false;
+
+    if (!observerList->zeroedObservers.isEmpty())
+        collectZeroedObservers(context);
+}
+
+NetworkStateNotifier::ObserverList* NetworkStateNotifier::lockAndFindObserverList(ExecutionContext* context)
+{
+    MutexLocker locker(m_mutex);
+    ObserverListMap::iterator it = m_observers.find(context);
+    return it == m_observers.end() ? 0 : it->value;
+}
+
+void NetworkStateNotifier::collectZeroedObservers(ExecutionContext* context)

[adamk, 2014/05/21 09:21:31]

Please add a context thread assertion here.

[jkarlin, 2014/05/21 14:23:17]

Done.

+{
+    // If any observers were removed during the iteration they will have
+    // 0 values, clean them up.
+    ObserverList* list = lockAndFindObserverList(context);

[adamk, 2014/05/21 09:21:31]

Maybe the callers should just pass in the list?

[jkarlin, 2014/05/21 14:23:17]

Done.

+    ASSERT(list);

[adamk, 2014/05/21 09:21:31]

I think you also want to ASSERT(!list->iterating)

[jkarlin, 2014/05/21 14:23:17]

Done.

+
+    for (size_t i = 0; i < list->zeroedObservers.size(); ++i)
+        list->observers.remove(list->zeroedObservers[i]);
+
+    list->zeroedObservers.clear();
+
+    {
+        MutexLocker locker(m_mutex);
+        if (list->observers.isEmpty()) {
+            m_observers.remove(context);
+            delete list;

[adamk, 2014/05/21 09:21:31]

This delete can also go away if you use OwnPtrs as values.

[jkarlin, 2014/05/21 14:23:17]

Done.

+        }
+    }
+}
+
+} // namespace WebCore