Summary: | ERROR: Unhandled web process message 'StorageAreaMap:DispatchStorageEvent' | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | andersca, commit-queue, darin, esprehn+autocc, gyuyoung.kim, japhet, kling, mcatanzaro | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Local Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | 150362 | ||||||||||
Bug Blocks: | |||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2015-10-18 23:27:40 PDT
Created attachment 263445 [details]
Patch
Created attachment 263455 [details]
Updated patch
Addressed all review comments.
Oops, wrong bug, sorry for the noise. Comment on attachment 263445 [details]
Patch
r=me
Committed r191333: <http://trac.webkit.org/changeset/191333> This caused multiple assertions on regression tests: https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r191333%20(482)/results.html Will roll out. Re-opened since this is blocked by bug 150362 Yes, I didn't take into account that now we can have session storage maps without a listener when the connection is closed and destroyStorageMap() hasn't been called. We could either simply change the ASSERT to something like ASSERT(isSessionStporage() || m_eventListeners.contains(std::make_pair(&connection, storageMapID))); or add checks to processDidCloseConnection() and applicationWillTerminate() in case of session storage to not call removeListener if the listener is not present. Since HashMap handles the case of removing a non exiting item, I think it's simpler and probably more efficient to change the assert. Although, it's true that it wouldn't catch valid cases of session storage maps that should have a listener at that point. What do you guys think? Created attachment 263812 [details]
Changed the assert
Ok, took the simplest approach and changed the assert. Just cq+ this if you agree with changing the assert.
Comment on attachment 263812 [details]
Changed the assert
Sure, this seems fine.
Comment on attachment 263812 [details] Changed the assert Clearing flags on attachment: 263812 Committed r191481: <http://trac.webkit.org/changeset/191481> All reviewed patches have been landed. Closing bug. *** Bug 149387 has been marked as a duplicate of this bug. *** |