Changeset 183185 in webkit

Timestamp:

Apr 23, 2015, 8:49:47 AM (11 years ago)

Author:

eric.carlson@apple.com

Message:

Some media tests assert after r183096
​https://bugs.webkit.org/show_bug.cgi?id=144098

Reviewed by Darin Adler.

• dom/Document.cpp:

(WebCore::Document::removePlaybackTargetPickerClient): Don't assert if the client has already

been removed from the map. This happens when a media element is removed from the document
before its destructor runs and is not an error.

(WebCore::Document::showPlaybackTargetPicker): It is an error to call this after the client

has been removed from the map so leave the assert in a debug build, but return early
so a release build doesn't crash.

(WebCore::Document::playbackTargetPickerClientStateDidChange): Ditto.
(WebCore::Document::playbackTargetAvailabilityDidChange): Use "auto" for the map iterator variable.
(WebCore::Document::setPlaybackTarget): Ditto.
(WebCore::Document::setShouldPlayToPlaybackTarget): Ditto.

• dom/Document.h: Fix map typedef names.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (5 diffs)
• dom/Document.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-04-23  Eric Carlson  <eric.carlson@apple.com>
+
+        Some media tests assert after r183096
+        https://bugs.webkit.org/show_bug.cgi?id=144098
+
+        Reviewed by Darin Adler.
+
+        * dom/Document.cpp:
+        (WebCore::Document::removePlaybackTargetPickerClient): Don't assert if the client has already
+            been removed from the map. This happens when a media element is removed from the document
+            before its destructor runs and is not an error.
+        (WebCore::Document::showPlaybackTargetPicker): It is an error to call this after the client 
+            has been removed from the map so leave the assert in a debug build, but return early
+            so a release build doesn't crash.
+        (WebCore::Document::playbackTargetPickerClientStateDidChange): Ditto.
+        (WebCore::Document::playbackTargetAvailabilityDidChange): Use "auto" for the map iterator variable.
+        (WebCore::Document::setPlaybackTarget): Ditto.
+        (WebCore::Document::setShouldPlayToPlaybackTarget): Ditto.
+        * dom/Document.h: Fix map typedef names.
+
 2015-04-23  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -6551 +6551 @@
 void Document::removePlaybackTargetPickerClient(MediaPlaybackTargetClient& client)
 {
-    ASSERT(m_clientToIDMap.contains(&client));
-    uint64_t contextId = m_clientToIDMap.get(&client);
-    m_idToClientMap.remove(contextId);
-    m_clientToIDMap.remove(&client);
+    const auto& it = m_clientToIDMap.find(&client);
+    if (it == m_clientToIDMap.end())
+        return;
+
+    uint64_t clientId = it->value;
+    m_idToClientMap.remove(clientId);
+    m_clientToIDMap.remove(it);
 
     Page* page = this->page();
     if (!page)
         return;
-    page->removePlaybackTargetPickerClient(contextId);
+    page->removePlaybackTargetPickerClient(clientId);
 }
 
@@ -6568 +6571 @@
         return;
 
-    ASSERT(m_clientToIDMap.contains(&client));
-    page->showPlaybackTargetPicker(m_clientToIDMap.get(&client), view()->lastKnownMousePosition(), isVideo);
+    const auto& it = m_clientToIDMap.find(&client);
+    ASSERT(it != m_clientToIDMap.end());
+    if (it == m_clientToIDMap.end())
+        return;
+
+    page->showPlaybackTargetPicker(it->value, view()->lastKnownMousePosition(), isVideo);
 }
 
@@ -6578 +6585 @@
         return;
 
-    ASSERT(m_clientToIDMap.contains(&client));
-    page->playbackTargetPickerClientStateDidChange(m_clientToIDMap.get(&client), state);
+    const auto& it = m_clientToIDMap.find(&client);
+    ASSERT(it != m_clientToIDMap.end());
+    if (it == m_clientToIDMap.end())
+        return;
+
+    page->playbackTargetPickerClientStateDidChange(it->value, state);
 }
 
 void Document::playbackTargetAvailabilityDidChange(uint64_t clientId, bool available)
 {
-    TargetClientToIdMap::iterator it = m_idToClientMap.find(clientId);
+    const auto& it = m_idToClientMap.find(clientId);
     if (it == m_idToClientMap.end())
         return;
@@ -6593 +6604 @@
 void Document::setPlaybackTarget(uint64_t clientId, Ref<MediaPlaybackTarget>&& target)
 {
-    TargetClientToIdMap::iterator it = m_idToClientMap.find(clientId);
+    const auto& it = m_idToClientMap.find(clientId);
     if (it == m_idToClientMap.end())
         return;
@@ -6602 +6613 @@
 void Document::setShouldPlayToPlaybackTarget(uint64_t clientId, bool shouldPlay)
 {
-    TargetClientToIdMap::iterator it = m_idToClientMap.find(clientId);
+    const auto& it = m_idToClientMap.find(clientId);
     if (it == m_idToClientMap.end())
         return;

trunk/Source/WebCore/dom/Document.h

@@ -1689 +1689 @@
 
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)
-    typedef HashMap<uint64_t, WebCore::MediaPlaybackTargetClient*> TargetClientToIdMap;
-    TargetClientToIdMap m_idToClientMap;
-    typedef HashMap<WebCore::MediaPlaybackTargetClient*, uint64_t> TargetIdToClientMap;
-    TargetIdToClientMap m_clientToIDMap;
+    typedef HashMap<uint64_t, WebCore::MediaPlaybackTargetClient*> TargetIdToClientMap;
+    TargetIdToClientMap m_idToClientMap;
+    typedef HashMap<WebCore::MediaPlaybackTargetClient*, uint64_t> TargetClientToIdMap;
+    TargetClientToIdMap m_clientToIDMap;
 #endif
 };