[v8] The last portion of CSS GC work: fixing fast/dom/StyleSheet/gc-stylesheet-wrapper.html
Created attachment 76691 [details] Patch
Comment on attachment 76691 [details] Patch WebCore uses 4 space indent. I'm not sure how this passed the style queue.
Comment on attachment 76691 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=76691&action=review > WebCore/bindings/v8/V8GCController.cpp:371 > + if (object) { Early return is strongly preferred in WebKit.
Created attachment 76701 [details] Patch
(In reply to comment #2) > (From update of attachment 76691 [details]) > WebCore uses 4 space indent. I'm not sure how this passed the style queue. Thanks a lot, Eric, fixed. I am surprised as well
Fixed too, but webkit-patch upload takes a lot of time on my MBP, please, wait for 3rd patch (In reply to comment #3) > (From update of attachment 76691 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=76691&action=review > > > WebCore/bindings/v8/V8GCController.cpp:371 > > + if (object) { > > Early return is strongly preferred in WebKit.
Created attachment 76705 [details] Patch
(In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 76691 [details] [details]) > > WebCore uses 4 space indent. I'm not sure how this passed the style queue. > > Thanks a lot, Eric, fixed. I am surprised as well check-webkit-style doesn't check indents really. It requires a bit of "intelligence" to do this correctly even at a simple level. For example, a(b, c d); Would be indented correctly even though d would not be indented at a multiple of 4. You need to detect a statement end basically and then check to see that the indent is a multiple of 4. (To check that it is the correct multiple of 4 is more complicated because there are special rules for switch (for example).)
Comment on attachment 76705 [details] Patch Don't you need to remove it from the skipped list? Looks sane enough to me, but I don't fully understand the consequences of this change not being particularly well versed in V8.
Mads, can you do an unofficial review of this please?
This looks good to me.
Thanks a lot, Mads. Guys, may I get r+, land this thing and finish CSS GC cleanup? (In reply to comment #11) > This looks good to me.
Comment on attachment 76705 [details] Patch Clearing flags on attachment: 76705 Committed r74260: <http://trac.webkit.org/changeset/74260>
All reviewed patches have been landed. Closing bug.