Random Gamepad cleanup
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 Aug 2014 20:39:07 +0000 (20:39 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 23 Aug 2014 20:39:07 +0000 (20:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=136193

Reviewed by Sam Weinig.

No new tests (Not tested yet, and no change in behavior anyways)

- Use DOMWindow WeakPtrs when iterating
- More references instead of pointers

* Modules/gamepad/GamepadEvent.cpp:
(WebCore::GamepadEvent::GamepadEvent):
* Modules/gamepad/GamepadEvent.h:
(WebCore::GamepadEvent::create):

* Modules/gamepad/GamepadManager.cpp:
(WebCore::GamepadManager::platformGamepadDisconnected):
(WebCore::GamepadManager::makeGamepadVisible):
* Modules/gamepad/NavigatorGamepad.cpp:
(WebCore::NavigatorGamepad::gamepadFromPlatformGamepad):
(WebCore::NavigatorGamepad::gamepadAtIndex): Deleted.
* Modules/gamepad/NavigatorGamepad.h:

* page/DOMWindow.cpp:
(WebCore::DOMWindow::DOMWindow):
* page/DOMWindow.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@172890 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/gamepad/GamepadEvent.cpp
Source/WebCore/Modules/gamepad/GamepadEvent.h
Source/WebCore/Modules/gamepad/GamepadManager.cpp
Source/WebCore/Modules/gamepad/NavigatorGamepad.cpp
Source/WebCore/Modules/gamepad/NavigatorGamepad.h
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h

index b841691..9b1a89e 100644 (file)
@@ -1,3 +1,32 @@
+2014-08-23  Brady Eidson  <beidson@apple.com>
+
+        Random Gamepad cleanup
+        https://bugs.webkit.org/show_bug.cgi?id=136193
+
+        Reviewed by Sam Weinig.
+
+        No new tests (Not tested yet, and no change in behavior anyways)
+
+        - Use DOMWindow WeakPtrs when iterating
+        - More references instead of pointers
+
+        * Modules/gamepad/GamepadEvent.cpp:
+        (WebCore::GamepadEvent::GamepadEvent):
+        * Modules/gamepad/GamepadEvent.h:
+        (WebCore::GamepadEvent::create):
+
+        * Modules/gamepad/GamepadManager.cpp:
+        (WebCore::GamepadManager::platformGamepadDisconnected):
+        (WebCore::GamepadManager::makeGamepadVisible):
+        * Modules/gamepad/NavigatorGamepad.cpp:
+        (WebCore::NavigatorGamepad::gamepadFromPlatformGamepad):
+        (WebCore::NavigatorGamepad::gamepadAtIndex): Deleted.
+        * Modules/gamepad/NavigatorGamepad.h:
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::DOMWindow):
+        * page/DOMWindow.h:
+
 2014-08-23  Byungseon Shin  <sun.shin@lge.com>
 
         Unify GraphicsLayer::setContentsToMedia and setContentsToCanvas
index 5badf46..d8ea72a 100644 (file)
@@ -33,9 +33,9 @@ GamepadEvent::GamepadEvent()
 {
 }
 
-GamepadEvent::GamepadEvent(const AtomicString& eventType, Gamepad* gamepad)
+GamepadEvent::GamepadEvent(const AtomicString& eventType, Gamepad& gamepad)
     : Event(eventType, false, false)
-    , m_gamepad(gamepad)
+    , m_gamepad(&gamepad)
 {
 }
 
index 29f706d..142d2e4 100644 (file)
@@ -50,7 +50,7 @@ public:
         return adoptRef(new GamepadEvent);
     }
 
-    static PassRefPtr<GamepadEvent> create(const AtomicString& eventType, Gamepad* gamepad)
+    static PassRefPtr<GamepadEvent> create(const AtomicString& eventType, Gamepad& gamepad)
     {
         return adoptRef(new GamepadEvent(eventType, gamepad));
     }
@@ -66,7 +66,7 @@ public:
 
 private:
     GamepadEvent();
-    explicit GamepadEvent(const AtomicString& eventType, Gamepad*);
+    explicit GamepadEvent(const AtomicString& eventType, Gamepad&);
     GamepadEvent(const AtomicString& eventType, const GamepadEventInit&);
 
     RefPtr<Gamepad> m_gamepad;
index 81bc95d..c3e086f 100644 (file)
@@ -77,18 +77,21 @@ void GamepadManager::platformGamepadConnected(PlatformGamepad& platformGamepad)
 
 void GamepadManager::platformGamepadDisconnected(PlatformGamepad& platformGamepad)
 {
-    Vector<DOMWindow*> domWindowVector;
-    copyToVector(m_domWindows, domWindowVector);
+    Vector<WeakPtr<DOMWindow>> weakWindows;
+    for (auto* domWindow : m_domWindows)
+        weakWindows.append(domWindow->createWeakPtr());
 
     HashSet<NavigatorGamepad*> notifiedNavigators;
 
     // Handle the disconnect for all DOMWindows with event listeners and their Navigators.
-    for (auto* window : domWindowVector) {
+    for (auto& window : weakWindows) {
         // Event dispatch might have made this window go away.
-        if (!m_domWindows.contains(window))
+        if (!window)
             continue;
 
-        NavigatorGamepad* navigator = navigatorGamepadFromDOMWindow(window);
+        // This DOMWindow's Navigator might not be accessible. e.g. The DOMWindow might be in the back/forward cache.
+        // If this happens the DOMWindow will not get this gamepaddisconnected event.
+        NavigatorGamepad* navigator = navigatorGamepadFromDOMWindow(window.get());
         if (!navigator)
             continue;
 
@@ -96,8 +99,7 @@ void GamepadManager::platformGamepadDisconnected(PlatformGamepad& platformGamepa
         if (m_gamepadBlindNavigators.contains(navigator))
             continue;
 
-        RefPtr<Gamepad> gamepad = navigator->gamepadAtIndex(platformGamepad.index());
-        ASSERT(gamepad);
+        Ref<Gamepad> gamepad(navigator->gamepadFromPlatformGamepad(platformGamepad));
 
         navigator->gamepadDisconnected(platformGamepad);
         notifiedNavigators.add(navigator);
@@ -132,21 +134,23 @@ void GamepadManager::makeGamepadVisible(PlatformGamepad& platformGamepad, HashSe
     for (auto* navigator : navigatorSet)
         navigator->gamepadConnected(platformGamepad);
 
-    Vector<DOMWindow*> domWindowVector;
-    copyToVector(domWindowSet, domWindowVector);
+    Vector<WeakPtr<DOMWindow>> weakWindows;
+    for (auto* domWindow : m_domWindows)
+        weakWindows.append(domWindow->createWeakPtr());
 
-    for (auto* window : domWindowVector) {
+    for (auto& window : weakWindows) {
         // Event dispatch might have made this window go away.
-        if (!m_domWindows.contains(window))
+        if (!window)
             continue;
 
-        NavigatorGamepad* navigator = navigatorGamepadFromDOMWindow(window);
+        // This DOMWindow's Navigator might not be accessible. e.g. The DOMWindow might be in the back/forward cache.
+        // If this happens the DOMWindow will not get this gamepadconnected event.
+        // The new gamepad will still be visibile to it once it is restored from the back/forward cache.
+        NavigatorGamepad* navigator = navigatorGamepadFromDOMWindow(window.get());
         if (!navigator)
             continue;
 
-        RefPtr<Gamepad> gamepad = navigator->gamepadAtIndex(platformGamepad.index());
-        ASSERT(gamepad);
-
+        Ref<Gamepad> gamepad(navigator->gamepadFromPlatformGamepad(platformGamepad));
         window->dispatchEvent(GamepadEvent::create(eventNames().gamepadconnectedEvent, gamepad.get()), window->document());
     }
 }
index 58a7ccb..a71ffa2 100644 (file)
@@ -72,11 +72,13 @@ NavigatorGamepad* NavigatorGamepad::from(Navigator* navigator)
     return supplement;
 }
 
-Gamepad* NavigatorGamepad::gamepadAtIndex(unsigned index)
+PassRef<Gamepad> NavigatorGamepad::gamepadFromPlatformGamepad(PlatformGamepad& platformGamepad)
 {
-    if (index >= m_gamepads.size())
-        return nullptr;
-    return m_gamepads[index].get();
+    unsigned index = platformGamepad.index();
+    if (index >= m_gamepads.size() || !m_gamepads[index])
+        return adoptRef(*Gamepad::create(platformGamepad).leakRef());
+
+    return *m_gamepads[index];
 }
 
 const Vector<RefPtr<Gamepad>>& NavigatorGamepad::getGamepads(Navigator* navigator)
index 734d787..e3690ce 100644 (file)
@@ -53,7 +53,7 @@ public:
     void gamepadConnected(PlatformGamepad&);
     void gamepadDisconnected(PlatformGamepad&);
 
-    Gamepad* gamepadAtIndex(unsigned index);
+    PassRef<Gamepad> gamepadFromPlatformGamepad(PlatformGamepad&);
 
 private:
     static const char* supplementName();
index ff39b94..fda9509 100644 (file)
@@ -393,6 +393,7 @@ DOMWindow::DOMWindow(Document* document)
     , m_shouldPrintWhenFinishedLoading(false)
     , m_suspendedForPageCache(false)
     , m_lastPageStatus(PageStatusNone)
+    , m_weakPtrFactory(this)
 #if PLATFORM(IOS)
     , m_scrollEventListenerCount(0)
 #endif
index 97a1552..e24c01c 100644 (file)
@@ -34,6 +34,7 @@
 #include "Supplementable.h"
 #include <functional>
 #include <memory>
+#include <wtf/WeakPtr.h>
 
 namespace Inspector {
 class ScriptCallStack;
@@ -445,6 +446,8 @@ namespace WebCore {
         void enableSuddenTermination();
         void disableSuddenTermination();
 
+        WeakPtr<DOMWindow> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
     private:
         explicit DOMWindow(Document*);
 
@@ -494,6 +497,8 @@ namespace WebCore {
         enum PageStatus { PageStatusNone, PageStatusShown, PageStatusHidden };
         PageStatus m_lastPageStatus;
 
+        WeakPtrFactory<DOMWindow> m_weakPtrFactory;
+
 #if PLATFORM(IOS)
         unsigned m_scrollEventListenerCount;
 #endif