We would like to update PRCList usage in nsPluginHost to use mozilla::LinkedList. - Convert |sListHead| [1], |sModuleListHead| [2] to be a mozilla::LinkedList. - Update |PluginDestructionGuard| [3], |PluginModuleMapping| [4] to inherit from mozilla::LinkedListElement - Update PRCList calls to LinkedList equivalents: - remove PR_INIT_CLIST - PR_INSERT_BEFORE -> LinkedList::insertBack - PR_INSERT_AFTER/PR_APPEND_LINK -> LinkedList::insertFront - remove PR_REMOVE_LINK - PR_CLIST_IS_EMPTY -> LinkedList::isEmpty - convert do/while loops w/ PR_LIST_HEAD/PR_NEXT_LINK -> range-based for (auto elm : list) [1] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/base/nsPluginHost.h#456 [2] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/ipc/PluginModuleParent.cpp#354 [3] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/base/nsPluginHost.h#419 [4] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/ipc/PluginModuleParent.cpp#216

This got a little mixed up, the proper names are: - Convert |sListHead| [1], |sRunnableListHead| [2] to be a mozilla::LinkedList. - Update |PluginDestructionGuard| [3], |nsPluginDestroyRunnable| [4] to inherit from mozilla::LinkedListElement [1] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/base/nsPluginHost.h#456 [2] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/base/nsPluginHost.cpp#4036 [3] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/base/nsPluginHost.h#419 [4] http://searchfox.org/mozilla-central/rev/214345204f1e7d97abb571b7992b6deedb5ff98f/dom/plugins/base/nsPluginHost.cpp#3977

Hello, I'd like to work on this bug

(In reply to Łukasz Kędziorski from comment #2) > Hello, I'd like to work on this bug Great! Let me know if you need any help getting started.

Comment on attachment 8863283 [details] [diff] [review] 1357578 v1 Review of attachment 8863283 [details] [diff] [review]: ----------------------------------------------------------------- Thank you Łukasz, this looks great! There are just few small changes I'd like you to make. ::: dom/plugins/base/nsPluginHost.cpp @@ +4005,5 @@ > } > > + nsPluginDestroyRunnable *r = sRunnableListHead.getFirst(); > + > + while (r != sRunnableListHead.getLast()->getNext()) { This can be simplified to a range-based for loop, ie: > for (auto r : sRunnableListHead) @@ +4031,4 @@ > protected: > RefPtr<nsNPAPIPluginInstance> mInstance; > > + static mozilla::LinkedList<nsPluginDestroyRunnable> sRunnableListHead; We can rename these lists now. The 'Head' portion isn't necessary, so just 'sRunnableList'. @@ +4084,5 @@ > // destroy upon destruction. > > + PluginDestructionGuard *g = sListHead.getFirst(); > + > + while (g != sListHead.getLast()->getNext()) { This can also be simplified to a range-based for loop: > for (auto g : sList) ::: dom/plugins/base/nsPluginHost.h @@ +6,4 @@ > #ifndef nsPluginHost_h_ > #define nsPluginHost_h_ > > +#include "mozilla/LinkedList.h" We can remove the "prclist.h" include as well now. @@ +452,5 @@ > > RefPtr<nsNPAPIPluginInstance> mInstance; > bool mDelayedDestroy; > > + static mozilla::LinkedList<PluginDestructionGuard> sListHead; This can be renamed to just 'sList'.

I did all changes, but due to removal of #include I had to modify PluginModuleMapping.

Comment on attachment 8863928 [details] [diff] [review] 1357578 v2 Review of attachment 8863928 [details] [diff] [review]: ----------------------------------------------------------------- This is super close, we just have a few minor changes. Thank you for fixing PluginModuleParent.cpp as well, but that code is going to be removed soon so we don't want to change it (bug 1357582). Can you please revert your changes to PluginModuleParent.cpp and just add a #include "prclist.h" to PluginModuleParent.cpp instead? I think that should fix your build issues. ::: dom/plugins/base/nsPluginHost.cpp @@ +4009,5 @@ > // There's another runnable scheduled to tear down > // instance. Let it do the job. > return NS_OK; > } > + r = r->getNext(); Sorry for not being clearer! You don't need this anymore. The range-based for loop [1] takes care of it for you. [1] http://en.cppreference.com/w/cpp/language/range-for @@ +4086,5 @@ > g->mDelayedDestroy = true; > > return true; > } > + g = g->getNext(); Same here.

Comment on attachment 8863946 [details] [diff] [review] 1357578 v3 Review of attachment 8863946 [details] [diff] [review]: ----------------------------------------------------------------- Thank you this looks perfect. We just need to get sign-off from a plugin peer now. bsemdberg, can you take a look at this. I've already gone through a few iterations and it looks good to me.

Comment on attachment 8863946 [details] [diff] [review] 1357578 v3 great, thank you for taking this Łukasz!

https://hg.mozilla.org/integration/mozilla-inbound/rev/e6c824580042a056a77eebd7edeb433ccd7b0ab9 Bug 1357578 - Convert PRCList usage in nsPluginHost to mozilla::LinkedList. r=erahm,r=bsemdberg