Bug 122496

Summary: Remove use of deprecatedDeleteAllValues in NPRemoteObjectMap::pluginDestroyed
Product: WebKit Reporter: Darin Adler <darin>
Component: WebKit2Assignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, commit-queue, gustavo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 122504, 122547    
Bug Blocks: 73757    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Darin Adler
Reported 2013-10-08 00:15:16 PDT
Remove use of deleteAllValues in NPRemoteObjectMap::pluginDestroyed
Attachments
Patch (4.79 KB, patch)
2013-10-08 00:17 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (5.01 KB, patch)
2013-10-09 09:41 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Patch (3.41 KB, patch)
2014-04-17 08:44 PDT, Darin Adler 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2013-10-08 00:17:01 PDT
Created attachment 213668 [details] Patch
Andreas Kling
Comment 2 2013-10-08 00:33:18 PDT
Comment on attachment 213668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=213668&action=review r=me, this is much nicer than the old code. Thought: Do you think it would be cool to have a function that yanks items matching a [lambda] predicate out of a collection and returns them in a Vector? > Source/WebKit2/ChangeLog:9 > + (WebKit::NPRemoteObjectMap::registerNPObject): Don't call release when puttin objects Typo, putting. > Source/WebKit2/Shared/Plugins/NPRemoteObjectMap.cpp:200 > + for (auto it = m_registeredNPObjects.begin(), end = m_registeredNPObjects.end(); it != end; ++it) { You can use range-based for in WebKit2. > Source/WebKit2/Shared/Plugins/NPRemoteObjectMap.cpp:209 > + for (auto it = m_npObjectProxies.begin(), end = m_npObjectProxies.end(); it != end; ++it) { Same here.
Darin Adler
Comment 3 2013-10-08 00:50:53 PDT
(In reply to comment #2) > Thought: Do you think it would be cool to have a function that yanks items matching a [lambda] predicate out of a collection and returns them in a Vector? Probably. It would use the word “take” in its name.
Darin Adler
Comment 4 2013-10-08 00:57:01 PDT
Committed r157090: <http://trac.webkit.org/changeset/157090>
Alexey Proskuryakov
Comment 5 2013-10-08 08:18:49 PDT
This caused crashes all over the place in plug-in tests. Darin, are you around to take a look?
WebKit Commit Bot
Comment 6 2013-10-08 08:24:18 PDT
Re-opened since this is blocked by bug 122504
Alexey Proskuryakov
Comment 7 2013-10-08 08:28:24 PDT
Rolled out in <http://trac.webkit.org/changeset/157105>. Test results: <http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r157090%20(12401)/results.html>. Unfortunately, there are no crash logs (might be that they are actually freezes incorrectly detected as crashes).
Darin Adler
Comment 8 2013-10-08 09:44:23 PDT
Were the crashes only on Mountain Lion, or were they on Lion too?
Alexey Proskuryakov
Comment 9 2013-10-08 09:56:22 PDT
Crashing on Lion too. Actually, that bot has crash logs - http://build.webkit.org/results/Apple%20Lion%20Debug%20WK2%20(Tests)/r157104%20(11972)/plugins/netscape-plugin-map-data-to-src-crash-log.txt
Darin Adler
Comment 10 2013-10-08 22:24:50 PDT
Committed r157157: <http://trac.webkit.org/changeset/157157>
Gustavo Noronha (kov)
Comment 11 2013-10-09 05:09:29 PDT
Looks like the crashes came back with the new commit =(
WebKit Commit Bot
Comment 12 2013-10-09 05:45:02 PDT
Re-opened since this is blocked by bug 122547
Darin Adler
Comment 14 2013-10-09 09:39:22 PDT
Really? I did lots of testing locally and saw no crashes. I guess I am testing this wrong. Anders, can you help?
Darin Adler
Comment 15 2013-10-09 09:41:05 PDT
Created attachment 213786 [details] Patch
Darin Adler
Comment 16 2013-10-09 09:41:38 PDT
Anders, any ideas on why this second version of the patch still caused all those crashes on the bots, but not on my Mavericks development machine?
Darin Adler
Comment 17 2014-04-17 08:44:28 PDT
Created attachment 229547 [details] Patch
Darin Adler
Comment 18 2014-04-17 09:59:02 PDT
Committed r167431: <http://trac.webkit.org/changeset/167431>
Note You need to log in before you can comment on or make changes to this bug.