Bug 122496 - Remove use of deprecatedDeleteAllValues in NPRemoteObjectMap::pluginDestroyed
Summary: Remove use of deprecatedDeleteAllValues in NPRemoteObjectMap::pluginDestroyed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on: 122504 122547
Blocks: 73757
  Show dependency treegraph
 
Reported: 2013-10-08 00:15 PDT by Darin Adler
Modified: 2014-04-17 09:59 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.79 KB, patch)
2013-10-08 00:17 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (5.01 KB, patch)
2013-10-09 09:41 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (3.41 KB, patch)
2014-04-17 08:44 PDT, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2013-10-08 00:15:16 PDT
Remove use of deleteAllValues in NPRemoteObjectMap::pluginDestroyed
Comment 1 Darin Adler 2013-10-08 00:17:01 PDT
Created attachment 213668 [details]
Patch
Comment 2 Andreas Kling 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.
Comment 3 Darin Adler 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.
Comment 4 Darin Adler 2013-10-08 00:57:01 PDT
Committed r157090: <http://trac.webkit.org/changeset/157090>
Comment 5 Alexey Proskuryakov 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?
Comment 6 WebKit Commit Bot 2013-10-08 08:24:18 PDT
Re-opened since this is blocked by bug 122504
Comment 7 Alexey Proskuryakov 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).
Comment 8 Darin Adler 2013-10-08 09:44:23 PDT
Were the crashes only on Mountain Lion, or were they on Lion too?
Comment 9 Alexey Proskuryakov 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
Comment 10 Darin Adler 2013-10-08 22:24:50 PDT
Committed r157157: <http://trac.webkit.org/changeset/157157>
Comment 11 Gustavo Noronha (kov) 2013-10-09 05:09:29 PDT
Looks like the crashes came back with the new commit =(
Comment 12 WebKit Commit Bot 2013-10-09 05:45:02 PDT
Re-opened since this is blocked by bug 122547
Comment 14 Darin Adler 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?
Comment 15 Darin Adler 2013-10-09 09:41:05 PDT
Created attachment 213786 [details]
Patch
Comment 16 Darin Adler 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?
Comment 17 Darin Adler 2014-04-17 08:44:28 PDT
Created attachment 229547 [details]
Patch
Comment 18 Darin Adler 2014-04-17 09:59:02 PDT
Committed r167431: <http://trac.webkit.org/changeset/167431>