LGTM. And thanks a lot. Just for my education, into which release it's going to be merged? yours, anton. On Mon, Aug 22, 2011 at 12:38 PM, <cevans@chromium.org> wrote: > Reviewers: antonm, > > Description: > Merge 93397 > BUG=76771 > > Please review this at http://codereview.chromium.org/7697013/ > > SVN Base: http://svn.webkit.org/repository/webkit/branches/chromium/835/ > > Affected files: > M LayoutTests/platform/chromium/test_expectations.txt > M Source/WebCore/bindings/v8/V8GCController.cpp > > > Index: LayoutTests/platform/chromium/test_expectations.txt > =================================================================== > --- LayoutTests/platform/chromium/test_expectations.txt (revision 93494) > +++ LayoutTests/platform/chromium/test_expectations.txt (working copy) > @@ -198,6 +198,15 @@ > // Would have to be implemented much differently to work in v8. > WONTFIX : fast/dom/gc-10.html = FAIL > > +// Proper retention of CSS objects is tricky. Disable the tests for now. > + > +BUGWK66377 : fast/dom/StyleSheet/gc-declaration-parent-rule.html = TEXT > +BUGWK66377 : fast/dom/StyleSheet/gc-inline-style-cssvalues.html = TEXT > +BUGWK66377 : fast/dom/StyleSheet/gc-parent-rule.html = TEXT > +BUGWK66377 : fast/dom/StyleSheet/gc-parent-stylesheet.html = TEXT > +BUGWK66377 : fast/dom/StyleSheet/gc-rule-children-wrappers.html = TEXT > +BUGWK66377 : fast/dom/StyleSheet/gc-styleheet-wrapper.xhtml = TEXT > + > // This fails because we're missing various useless apple-specific > // properties on the window object. > // This test also timeouts in Debug mode. > Index: Source/WebCore/bindings/v8/V8GCController.cpp > =================================================================== > --- Source/WebCore/bindings/v8/V8GCController.cpp (revision 93494) > +++ Source/WebCore/bindings/v8/V8GCController.cpp (working copy) > @@ -286,40 +286,6 @@ > return GroupId(root); > } > > -static GroupId calculateGroupId(StyleBase* styleBase) > -{ > - ASSERT(styleBase); > - StyleBase* current = styleBase; > - StyleSheet* styleSheet = 0; > - while (true) { > - // Special case: CSSStyleDeclarations might be either inline and in > this case > - // we need to group them with their node or regular ones. > - if (current->isMutableStyleDeclaration()) { > - CSSMutableStyleDeclaration* cssMutableStyleDeclaration = > static_cast<CSSMutableStyleDeclaration*>(current); > - if (cssMutableStyleDeclaration->isInlineStyleDeclaration()) > - return > calculateGroupId(cssMutableStyleDeclaration->node()); > - // Either we have no parent, or this parent is a CSSRule. > - ASSERT(cssMutableStyleDeclaration->parent() == > cssMutableStyleDeclaration->parentRule()); > - } > - > - if (current->isStyleSheet()) > - styleSheet = static_cast<StyleSheet*>(current); > - > - StyleBase* parent = current->parent(); > - if (!parent) > - break; > - current = parent; > - } > - > - if (styleSheet) { > - if (Node* ownerNode = styleSheet->ownerNode()) > - return calculateGroupId(ownerNode); > - return GroupId(styleSheet); > - } > - > - return GroupId(current); > -} > - > class GrouperVisitor : public DOMWrapperMap<Node>::Visitor, public > DOMWrapperMap<void>::Visitor { > public: > void visitDOMWrapper(DOMDataStore* store, Node* node, > v8::Persistent<v8::Object> wrapper) > @@ -347,49 +313,6 @@ > > void visitDOMWrapper(DOMDataStore* store, void* object, > v8::Persistent<v8::Object> wrapper) > { > - WrapperTypeInfo* typeInfo = V8DOMWrapper::domWrapperType(wrapper); > - > - if (typeInfo->isSubclass(&V8StyleSheetList::info)) { > - StyleSheetList* styleSheetList = > static_cast<StyleSheetList*>(object); > - GroupId groupId(styleSheetList); > - if (Document* document = styleSheetList->document()) > - groupId = GroupId(document); > - m_grouper.append(GrouperItem(groupId, wrapper)); > - > - } else if (typeInfo->isSubclass(&V8DOMImplementation::info)) { > - DOMImplementation* domImplementation = > static_cast<DOMImplementation*>(object); > - GroupId groupId(domImplementation); > - if (Document* document = domImplementation->document()) > - groupId = GroupId(document); > - m_grouper.append(GrouperItem(groupId, wrapper)); > - > - } else if (typeInfo->isSubclass(&V8StyleSheet::info) || > typeInfo->isSubclass(&V8CSSRule::info)) { > - > m_grouper.append(GrouperItem(calculateGroupId(static_cast<StyleBase*>(object)), > wrapper)); > - > - } else if (typeInfo->isSubclass(&V8CSSStyleDeclaration::info)) { > - CSSStyleDeclaration* cssStyleDeclaration = > static_cast<CSSStyleDeclaration*>(object); > - > - GroupId groupId = calculateGroupId(cssStyleDeclaration); > - m_grouper.append(GrouperItem(groupId, wrapper)); > - > - // Keep alive "dirty" primitive values (i.e. the ones that > - // have user-added properties) by creating implicit > - // references between the style declaration and the values > - // in it. > - if (cssStyleDeclaration->isMutableStyleDeclaration()) { > - CSSMutableStyleDeclaration* cssMutableStyleDeclaration = > static_cast<CSSMutableStyleDeclaration*>(cssStyleDeclaration); > - Vector<v8::Persistent<v8::Value> > values; > - > values.reserveCapacity(cssMutableStyleDeclaration->length()); > - CSSMutableStyleDeclaration::const_iterator end = > cssMutableStyleDeclaration->end(); > - for (CSSMutableStyleDeclaration::const_iterator it = > cssMutableStyleDeclaration->begin(); it != end; ++it) { > - v8::Persistent<v8::Object> value = > store->domObjectMap().get(it->value()); > - if (!value.IsEmpty() && value->IsDirty()) > - values.append(value); > - } > - if (!values.isEmpty()) > - v8::V8::AddImplicitReferences(wrapper, values.data(), > values.size()); > - } > - } > } > > void applyGrouping() > > >
Chromium Code Reviews
