Right now HTMLCollection forwards ref/deref to its owner but this model makes tricky to merge it with DynamicNodeList. Also, we never delete HTMLCollection as long as its owner is alive because no one clears entries in m_cachedCollections, end up consuming a lot of memory over time when the node is left in the tree.
Created attachment 150507 [details] Cleanup
Comment on attachment 150507 [details] Cleanup Attachment 150507 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13125324
Comment on attachment 150507 [details] Cleanup Attachment 150507 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13138187
Created attachment 150514 [details] Fixed builds
Comment on attachment 150514 [details] Fixed builds Attachment 150514 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13126334
Comment on attachment 150514 [details] Fixed builds View in context: https://bugs.webkit.org/attachment.cgi?id=150514&action=review > Source/WebKit/win/DOMHTMLClasses.cpp:713 > + RefPtr<HTMLCollection> options = selectElement->options(); Oops, I need to change this to HTMLOptionsCollection.
Ping reviewers.
Comment on attachment 150514 [details] Fixed builds View in context: https://bugs.webkit.org/attachment.cgi?id=150514&action=review > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:122 > + return toJS(exec, thisObj, WTF::getPtr(collection)); Could you use release here? > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:80 > + return toJS(exec, thisObj->globalObject(), WTF::getPtr(collection)); Could you use release here?
(In reply to comment #8) > (From update of attachment 150514 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150514&action=review > > > Source/WebCore/bindings/js/JSDOMWindowCustom.cpp:122 > > + return toJS(exec, thisObj, WTF::getPtr(collection)); > > Could you use release here? > > > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:80 > > + return toJS(exec, thisObj->globalObject(), WTF::getPtr(collection)); > > Could you use release here? I don't think so. At least I don't see such a pattern anywhere else in the binding code.
Committed r122115: <http://trac.webkit.org/changeset/122115>
EFL build fix landed in http://trac.webkit.org/changeset/122118.
Another EFL build fix (microdata tests): http://trac.webkit.org/changeset/122155
I suspect this may have caused the WK1 Mac release bot to explode into flames: http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r122115%20(900)/results.html
(In reply to comment #13) > I suspect this may have caused the WK1 Mac release bot to explode into flames: > > http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r122115%20(900)/results.html That seems unlikely. I use WK1 Mac debug for development, and didn't see any failures like that when I ran the test locally. Is it possible that the bot is sick (some stale file lock, process, etc...) ?
(In reply to comment #14) > (In reply to comment #13) > > I suspect this may have caused the WK1 Mac release bot to explode into flames: > > > > http://build.webkit.org/results/Apple%20Lion%20Release%20WK1%20(Tests)/r122115%20(900)/results.html > > That seems unlikely. I use WK1 Mac debug for development, and didn't see any failures like that when I ran the test locally. > > Is it possible that the bot is sick (some stale file lock, process, etc...) ? Absolutely! I'm checking this now. Don't worry, I'm not hovering over the rollout trigger. ;-)