| Summary: | Crash loading skydrive.com (assertion under RemoteLayerTreeDisplayRefreshMonitor) | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | WebKit2 | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, simon.fraser | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2014-05-29 00:12:41 PDT
Created attachment 232236 [details]
patch
Comment on attachment 232236 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=232236&action=review > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:333 > + HashSet<RemoteLayerTreeDisplayRefreshMonitor*> monitors = m_displayRefreshMonitors; I think you should use a Vector and copy the hash set to it instead, that might be faster! Reopening to address post-landing review comments. Created attachment 232272 [details]
patch
Comment on attachment 232272 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=232272&action=review > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:343 > + HashSet<RemoteLayerTreeDisplayRefreshMonitor*> monitorsToNotify = m_displayRefreshMonitors; > + m_displayRefreshMonitorsToNotify = &monitorsToNotify; > + while (!monitorsToNotify.isEmpty()) > + monitorsToNotify.takeAny()->didUpdateLayers(); > + m_displayRefreshMonitorsToNotify = nullptr; > } Because takeAny() uses the begin() iterator, it has to iterate a chunk of the hash table each time to find the first non-null bucket. So, switching to takeAny() may not be a performance improvement. Comment on attachment 232272 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=232272&action=review >> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:343 >> } > > Because takeAny() uses the begin() iterator, it has to iterate a chunk of the hash table each time to find the first non-null bucket. So, switching to takeAny() may not be a performance improvement. The change is for correctness, not performance improvement. Comment on attachment 232272 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=232272&action=review > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:339 > + m_displayRefreshMonitorsToNotify = &monitorsToNotify; If would be good to ASSERT(!m_displayRefreshMonitorsToNotify) here. |