Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(161)

Issue 7697013: Merge 93397 (Closed)

Created:
9 years, 4 months ago by Chris Evans
Modified:
9 years, 4 months ago
Reviewers:
antonm
CC:
chromium-reviews
Base URL:
http://svn.webkit.org/repository/webkit/branches/chromium/835/
Visibility:
Public.

Description

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -77 lines) Patch
M LayoutTests/platform/chromium/test_expectations.txt View 1 chunk +9 lines, -0 lines 0 comments Download
M Source/WebCore/bindings/v8/V8GCController.cpp View 2 chunks +0 lines, -77 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Chris Evans
9 years, 4 months ago (2011-08-22 08:38:56 UTC) #1
antonm
9 years, 4 months ago (2011-08-22 13:45:27 UTC) #2
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()
>
>
>

Powered by Google App Engine
This is Rietveld 408576698