Bug 156648 – REGRESSION( r199603): Pandora crashes WebKit in WebPlaybackSessionManager::removeClientForContext

• Component: New Bugs
• Status: RESOLVED FIXED

[Jer Noble]

REGRESSION( r199603): Pandora crashes WebKit in WebPlaybackSessionManager::removeClientForContext

[Jer Noble]

rdar://problem/25758117

[Jer Noble]

Created attachment 276526 [details]
Patch

[Jer Noble]

Committed r199615: <http://trac.webkit.org/changeset/199615>

[Darin Adler]

Comment on attachment 276526 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276526&action=review

> Source/WebKit2/WebProcess/cocoa/WebPlaybackSessionManager.mm:262
> +    if (!m_mediaElements.contains(&mediaElement))
>          return;
>  
>      uint64_t contextId = m_mediaElements.get(&mediaElement);

This is an inefficient coding pattern. It’s a double hash table lookup. Preferred style is to take advantage of the empty value, and so know that a contextId of 0 means that mediaElement is not in m_mediaElement, or to use find and thus be able to do both the contains (iterator != m_mediaElement.end()) and get (iterator->value) operations and share that single hash table lookup.

[Jer Noble]

Reopening to attach new patch.

[Jer Noble]

Created attachment 276641 [details]
Follow up patch

[Darin Adler]

Comment on attachment 276641 [details]
Follow up patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276641&action=review

> Source/WebKit2/WebProcess/cocoa/WebPlaybackSessionManager.mm:236
> +    auto foundIter = m_mediaElements.find(&mediaElement);

Normally best not to abbreviate "iterator" as "iter". I would probably have named this just "iterator", but "foundIterator" is OK.

[Jer Noble]

Committed r199671: <http://trac.webkit.org/changeset/199671>