Bug 156648 - REGRESSION( r199603): Pandora crashes WebKit in WebPlaybackSessionManager::removeClientForContext
Summary: REGRESSION( r199603): Pandora crashes WebKit in WebPlaybackSessionManager::re...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jer Noble
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-04-15 16:57 PDT by Jer Noble
Modified: 2016-04-18 09:04 PDT (History)
2 users (show)

See Also:

Attachments
Patch (1.62 KB, patch)
2016-04-15 16:59 PDT, Jer Noble 		bdakin: review+
Details | Formatted Diff | Diff
Follow up patch (2.54 KB, patch)
2016-04-18 08:33 PDT, Jer Noble 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jer Noble 2016-04-15 16:57:41 PDT 
REGRESSION( r199603): Pandora crashes WebKit in WebPlaybackSessionManager::removeClientForContext
Comment 1 Jer Noble 2016-04-15 16:58:14 PDT 
rdar://problem/25758117
Comment 2 Jer Noble 2016-04-15 16:59:30 PDT 
Created attachment 276526 [details]
Patch
Comment 3 Jer Noble 2016-04-15 17:01:52 PDT 
Committed r199615: <http://trac.webkit.org/changeset/199615>
Comment 4 Darin Adler 2016-04-15 22:41:14 PDT 
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.
Comment 5 Jer Noble 2016-04-18 08:33:06 PDT 
Reopening to attach new patch.
Comment 6 Jer Noble 2016-04-18 08:33:14 PDT 
Created attachment 276641 [details]
Follow up patch
Comment 7 Darin Adler 2016-04-18 08:52:40 PDT 
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.
Comment 8 Jer Noble 2016-04-18 09:04:02 PDT 
Committed r199671: <http://trac.webkit.org/changeset/199671>