Summary: | The memory of FloatingObject should be handled automatically | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Benjamin Poulain <benjamin> | ||||||
Component: | Layout and Rendering | Assignee: | Benjamin Poulain <benjamin> | ||||||
Status: | NEW --- | ||||||||
Severity: | Normal | CC: | darin, mjs, mrowe | ||||||
Priority: | P3 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | 55602 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Benjamin Poulain
2011-03-02 13:38:08 PST
(In reply to comment #0) > OwnPtr won't do it Why? Maybe we can specialize the template to do something simple with OwnPtr. > maybe PassOwnPtr in the structure could do the trick Unlikely. Created attachment 85811 [details]
Patch
First step, make ListHashSet more flexible to be able to use Type and PassType with the structure.
Created attachment 85839 [details]
Patch
First step, make ListHashSet more flexible to be able to use Type and PassType with the structure. Same patch without French-inspired words in the changlog :)
Comment on attachment 85839 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=85839&action=review review- because the change to EventQueue is incorrect > Source/JavaScriptCore/ChangeLog:27 > + (WTF::ListHashSetNode::ListHashSetNode): > + (WTF::ListHashSetTranslator::hash): > + (WTF::ListHashSetTranslator::equal): > + (WTF::ListHashSetTranslator::translate): > + (WTF::::find): > + (WTF::::contains): > + (WTF::::add): > + (WTF::::insertBefore): > + (WTF::::remove): It would be better to add comments explaining what’s changing in these functions. Also, the script generates “WTF::::find”, which is nonsense. Please fix that rather than checking it in. > Source/JavaScriptCore/wtf/ListHashSet.h:103 > + template<typename ArgType> iterator find(const ArgType&); > + template<typename ArgType> const_iterator find(const ArgType&) const; > + template<typename ArgType> bool contains(const ArgType&) const; This is stylistically different than the approach used in HashSet; we don’t provide a translator so this works only if the type is assignment-compatible. I don’t think it’s good for HashSet and ListHashSet to take different approaches to the same problem. HashSet also uses T for the type name and here you used the name ArgType. Lets do this in a way that’s consistent if possible. > Source/WebCore/ChangeLog:6 > + The memory of FloatingObject should be handled automatically > + https://bugs.webkit.org/show_bug.cgi?id=55611 I don’t think it makes sense to have the bug mention FloatingObject when this patch is just about extending ListHashSet. You should make a new bugs.webkit.org entry for just adding the new feature and then you can use a blocking relationship. It’s best not to use one bug to track two changes. > Source/WebCore/dom/EventQueue.cpp:112 > + if (event == lastPendingEvent) > + break; This won’t work if cancelEvent is called, passing in lastPendingEvent, during the execution of the event dispatch. |