Bug 50828 – [v8] More CSS wrappers GC work: keep document's style sheets, style sheet and css rule lists' items in proper object groups

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[anton muhin]

[v8] More CSS wrappers GC work: keep document's style sheets, style sheet and css rule lists' items in proper object groups

[anton muhin]

Created attachment 76238 [details]
Patch

[WebKit Review Bot]

Attachment 76238 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/dom/StyleSheet/gc-rule-children-wrappers-expected.txt', u'LayoutTests/fast/dom/StyleSheet/gc-rule-children-wrappers.html', u'WebCore/ChangeLog', u'WebCore/bindings/v8/DOMData.h', u'WebCore/bindings/v8/DOMDataStore.h', u'WebCore/bindings/v8/V8DOMMap.cpp', u'WebCore/bindings/v8/V8DOMMap.h', u'WebCore/bindings/v8/V8GCController.cpp']" exit_code: 1
WebCore/bindings/v8/V8DOMMap.h:39:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 76238 [details]
Patch

Thanks for working on these bugs!  I caused some of these problems but I couldn't figure out how to fix them.

[Alexey Proskuryakov]

Are you positive that the new subtests belong to gc-rule-children-wrappers? Are they about rule children wrappers?

Please don't land unless the answer is yes.

[Adam Barth]

I'm not sure I understand your question.  If you'd prefer the new tests to be in a separate file, we can certainly do that.

[Alexey Proskuryakov]

I didn't try very hard to understand what the new tests do - but yes, it seems like they do not test "rule children wrappers" behavior, and need to be in a different file.

[anton muhin]

Adam, Alexey, thanks a lot for comments.

I'll revert my changes to layout tests for now and would add another layout test later.

(In reply to comment #6)
> I didn't try very hard to understand what the new tests do - but yes, it seems like they do not test "rule children wrappers" behavior, and need to be in a different file.

[anton muhin]

Created attachment 76366 [details]
Patch

[WebKit Review Bot]

Attachment 76366 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/bindings/v8/DOMData.h', u'WebCore/bindings/v8/DOMDataStore.h', u'WebCore/bindings/v8/V8DOMMap.cpp', u'WebCore/bindings/v8/V8DOMMap.h', u'WebCore/bindings/v8/V8GCController.cpp']" exit_code: 1
WebCore/bindings/v8/V8DOMMap.h:39:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Pavel Feldman]

Comment on attachment 76366 [details]
Patch

Rubber stamping revert.

[WebKit Review Bot]

Comment on attachment 76366 [details]
Patch

Clearing flags on attachment: 76366

Committed r73907: <http://trac.webkit.org/changeset/73907>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.