Bug 109777 - ShadowRoot needs guardRef() and guardDeref()
Summary: ShadowRoot needs guardRef() and guardDeref()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
: 108710 108713 (view as bug list)
Depends on: 110766
Blocks: 72352 108713
  Show dependency treegraph
 
Reported: 2013-02-13 17:58 PST by Hajime Morrita
Modified: 2013-12-16 15:53 PST (History)
12 users (show)

See Also:


Attachments
Patch (13.89 KB, patch)
2013-02-13 22:41 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (14.79 KB, patch)
2013-02-13 23:32 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
For re-executing the test (14.86 KB, patch)
2013-02-14 01:12 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Fixed crashes (19.77 KB, patch)
2013-02-14 21:42 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (22.85 KB, patch)
2013-02-19 01:02 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (23.05 KB, patch)
2013-02-20 02:15 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Fixing EFL build failure (23.34 KB, patch)
2013-02-20 18:40 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (23.24 KB, patch)
2013-02-22 01:15 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (23.24 KB, patch)
2013-02-23 00:40 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch for landing (24.08 KB, patch)
2013-03-04 20:04 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2013-02-13 17:58:41 PST
After http://trac.webkit.org/changeset/137524, ShadowRoot has similar lifecycle characteristics to one of Document's:
It needs to live even after refcount goes zero. For this purpose, ShadowRoot needs guardRef and guardDeref() and
Scope change code needs to take care of that.
Comment 1 Hajime Morrita 2013-02-13 22:41:59 PST
Created attachment 188264 [details]
Patch
Comment 2 Build Bot 2013-02-13 23:21:41 PST
Comment on attachment 188264 [details]
Patch

Attachment 188264 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16559342
Comment 3 Hajime Morrita 2013-02-13 23:32:37 PST
Created attachment 188268 [details]
Patch
Comment 4 WebKit Review Bot 2013-02-14 00:41:10 PST
Comment on attachment 188268 [details]
Patch

Attachment 188268 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16542470

New failing tests:
accessibility/aria-checkbox-checked.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/cyrillic-uri.html
accessibility/anchor-linked-anonymous-block-crash.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
http/tests/appcache/deferred-events-delete-while-raising-timer.html
accessibility/aria-describedby-on-input.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-border-overflow.html
animations/animation-direction-alternate-reverse.html
accessibility/accessibility-object-detached.html
http/tests/appcache/abort-cache-ondownloading-manifest-404.html
http/tests/appcache/credential-url.html
http/tests/appcache/access-via-redirect.php
animations/3d/change-transform-in-end-event.html
accessibility/aria-controls-with-tabs.html
http/tests/appcache/destroyed-frame.html
animations/animation-controller-drt-api.html
animations/3d/transform-perspective.html
accessibility/accessibility-node-reparent.html
http/tests/appcache/deferred-events.html
animations/3d/state-at-end-event-transform.html
animations/animation-direction-normal.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 5 Hajime Morrita 2013-02-14 01:12:57 PST
Created attachment 188281 [details]
For re-executing the test
Comment 6 WebKit Review Bot 2013-02-14 02:19:44 PST
Comment on attachment 188281 [details]
For re-executing the test

Attachment 188281 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16546559

New failing tests:
accessibility/aria-checkbox-checked.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/cyrillic-uri.html
accessibility/anchor-linked-anonymous-block-crash.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
http/tests/appcache/deferred-events-delete-while-raising-timer.html
accessibility/aria-describedby-on-input.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-border-overflow.html
animations/animation-direction-alternate-reverse.html
accessibility/accessibility-object-detached.html
http/tests/appcache/abort-cache-ondownloading-manifest-404.html
http/tests/appcache/credential-url.html
animations/animation-controller-drt-api.html
animations/3d/change-transform-in-end-event.html
accessibility/aria-controls-with-tabs.html
canvas/philip/tests/2d.canvas.readonly.html
animations/3d/transform-perspective.html
accessibility/accessibility-node-reparent.html
http/tests/appcache/access-via-redirect.php
http/tests/appcache/deferred-events.html
animations/3d/state-at-end-event-transform.html
animations/animation-direction-normal.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 7 WebKit Review Bot 2013-02-14 03:21:28 PST
Comment on attachment 188281 [details]
For re-executing the test

Attachment 188281 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16581236

New failing tests:
accessibility/aria-checkbox-checked.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/cyrillic-uri.html
accessibility/anchor-linked-anonymous-block-crash.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
http/tests/appcache/deferred-events-delete-while-raising-timer.html
accessibility/aria-describedby-on-input.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-border-overflow.html
animations/animation-direction-alternate-reverse.html
accessibility/accessibility-object-detached.html
http/tests/appcache/abort-cache-ondownloading-manifest-404.html
http/tests/appcache/credential-url.html
http/tests/appcache/access-via-redirect.php
animations/3d/change-transform-in-end-event.html
accessibility/aria-controls-with-tabs.html
http/tests/appcache/destroyed-frame.html
animations/animation-controller-drt-api.html
animations/3d/transform-perspective.html
accessibility/accessibility-node-reparent.html
http/tests/appcache/deferred-events.html
animations/3d/state-at-end-event-transform.html
animations/animation-direction-normal.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 8 Hajime Morrita 2013-02-14 21:42:53 PST
Created attachment 188477 [details]
Fixed crashes
Comment 9 WebKit Review Bot 2013-02-14 23:55:09 PST
Comment on attachment 188477 [details]
Fixed crashes

Attachment 188477 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16570710

New failing tests:
accessibility/aria-checkbox-checked.html
animations/3d/matrix-transform-type-animation.html
http/tests/appcache/cyrillic-uri.html
accessibility/anchor-linked-anonymous-block-crash.html
animations/animation-add-events-in-handler.html
http/tests/appcache/deferred-events-delete-while-raising.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
http/tests/appcache/deferred-events-delete-while-raising-timer.html
accessibility/aria-describedby-on-input.html
http/tests/appcache/crash-when-navigating-away-then-back.html
animations/animation-border-overflow.html
animations/animation-direction-alternate-reverse.html
accessibility/accessibility-object-detached.html
http/tests/appcache/abort-cache-ondownloading-manifest-404.html
http/tests/appcache/credential-url.html
http/tests/appcache/access-via-redirect.php
animations/3d/change-transform-in-end-event.html
accessibility/aria-controls-with-tabs.html
http/tests/appcache/destroyed-frame.html
animations/animation-controller-drt-api.html
animations/3d/transform-perspective.html
accessibility/accessibility-node-reparent.html
http/tests/appcache/deferred-events.html
animations/3d/state-at-end-event-transform.html
animations/animation-direction-normal.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 10 Build Bot 2013-02-15 00:09:31 PST
Comment on attachment 188477 [details]
Fixed crashes

Attachment 188477 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16559831

New failing tests:
accessibility/anchor-linked-anonymous-block-crash.html
compositing/absolute-inside-out-of-view-fixed.html
animations/3d/matrix-transform-type-animation.html
http/tests/cache/cancel-multiple-post-xhrs.html
animations/3d/state-at-end-event-transform.html
animations/animation-add-events-in-handler.html
animations/additive-transform-animations.html
animations/3d/replace-filling-transform.html
http/tests/cache/history-only-cached-subresource-loads.html
compositing/bounds-in-flipped-writing-mode.html
accessibility/accessibility-node-reparent.html
animations/animation-border-overflow.html
accessibility/accessibility-object-detached.html
accessibility/anonymous-render-block-in-continuation-causes-crash.html
animations/animation-controller-drt-api.html
http/tests/cache/loaded-from-cache-after-reload-within-iframe.html
compositing/absolute-position-changed-with-composited-parent-layer.html
compositing/absolute-position-changed-in-composited-layer.html
http/tests/cache/iframe-304-crash.html
animations/3d/transform-perspective.html
http/tests/cache/cancel-during-failure-crash.html
canvas/philip/tests/2d.canvas.reference.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.5.html
canvas/philip/tests/2d.clearRect.basic.html
animations/3d/transform-origin-vs-functions.html
accessibility/accessibility-node-memory-management.html
animations/animation-css-rule-types.html
http/tests/cache/cached-main-resource.html
canvas/philip/tests/2d.clearRect+fillRect.basic.html
canvas/philip/tests/2d.clearRect+fillRect.alpha0.html
Comment 11 Hajime Morrita 2013-02-19 01:02:30 PST
Created attachment 189015 [details]
Patch
Comment 12 EFL EWS Bot 2013-02-19 03:21:13 PST
Comment on attachment 189015 [details]
Patch

Attachment 189015 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16628422
Comment 13 Elliott Sprehn 2013-02-19 15:10:01 PST
Comment on attachment 189015 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=189015&action=review

Overall this looks okay, I don't like the special method for updating the scope. Just add a check in setTreeScope and call that.

What's the long term plan for all this btw? Is haraken going to fix our if ancestor then child GC behavior? The guard refs appear to just be a hack around that.

> Source/WebCore/ChangeLog:23
> +        - Introduced setTreeScopeOfThisScope() which set Node::m_treeScope without chaning scope's m_guardRefCount.

changing

> Source/WebCore/dom/Document.h:1582
> +    scope->guardRef();

if (scope != this) scope->guardRef();

> Source/WebCore/dom/Document.h:1587
> +inline void Node::setTreeScopeOfThisScope(TreeScope* scope)

This method is confusing.

> Source/WebCore/dom/DocumentFragment.cpp:41
> +    ASSERT(document);

Is there a reason for this change?

> Source/WebCore/dom/TreeScope.cpp:105
> +    m_rootNode->setTreeScopeOfThisScope(noDocumentInstance());

Clever.

> Source/WebCore/dom/TreeScope.cpp:119
> +    rootNode()->m_inRemovedLastRefFunction = false;

This seems like a bad place to do this since the value depends on if you call SuperClass::dispose() before or after in your override.

> Source/WebCore/dom/TreeScope.h:107
> +    // Nodes belonging to this document hold guard references -

not a document. Fix the comment.

> Source/WebCore/editing/RemoveNodeCommand.cpp:29
> +#include "Document.h"

This seems like it's unrelated.
Comment 14 Hajime Morrita 2013-02-20 01:35:53 PST
Comment on attachment 189015 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=189015&action=review

>> Source/WebCore/ChangeLog:23
>> +        - Introduced setTreeScopeOfThisScope() which set Node::m_treeScope without chaning scope's m_guardRefCount.
> 
> changing

Fixed.

>> Source/WebCore/dom/Document.h:1582
>> +    scope->guardRef();
> 
> if (scope != this) scope->guardRef();

The condition is rejected by the caller side. 
Anyway, I'll do this ref/unref on caller side to avoid confusion.

>> Source/WebCore/dom/Document.h:1587
>> +inline void Node::setTreeScopeOfThisScope(TreeScope* scope)
> 
> This method is confusing.

Maybe. Will remove.

>> Source/WebCore/dom/DocumentFragment.cpp:41
>> +    ASSERT(document);
> 
> Is there a reason for this change?

Yep. Now ShadowRoot, a subclass of DocumentFragment, passes null document to DocumentFragment constructor.
And the ShadowRoot change is to align what Document is doing.

>> Source/WebCore/dom/TreeScope.cpp:119
>> +    rootNode()->m_inRemovedLastRefFunction = false;
> 
> This seems like a bad place to do this since the value depends on if you call SuperClass::dispose() before or after in your override.

Right. Will move to caller side.

>> Source/WebCore/dom/TreeScope.h:107
>> +    // Nodes belonging to this document hold guard references -
> 
> not a document. Fix the comment.

Woa good catch! Will do.

>> Source/WebCore/editing/RemoveNodeCommand.cpp:29
>> +#include "Document.h"
> 
> This seems like it's unrelated.

Well, I inlined moved some function to Document.h.
The linker told that this file uses it somewhere.
Comment 15 Hajime Morrita 2013-02-20 02:15:43 PST
Created attachment 189268 [details]
Patch
Comment 16 EFL EWS Bot 2013-02-20 03:13:53 PST
Comment on attachment 189268 [details]
Patch

Attachment 189268 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/16652212
Comment 17 Hajime Morrita 2013-02-20 18:40:10 PST
Created attachment 189442 [details]
Fixing EFL build failure
Comment 18 Elliott Sprehn 2013-02-20 19:38:05 PST
Comment on attachment 189442 [details]
Fixing EFL build failure

View in context: https://bugs.webkit.org/attachment.cgi?id=189442&action=review

Couple nits, but otherwise looks good.

> Source/WebCore/dom/TreeScope.h:134
> +    virtual void dispose() { }

I'd make this private.

> Source/WebCore/dom/TreeScopeAdopter.h:48
> +    void setNewScopeTo(Node*) const;

Lets rename this updateTreeScope(Node*)
Comment 19 Hajime Morrita 2013-02-22 01:15:01 PST
Created attachment 189712 [details]
Patch
Comment 20 Hajime Morrita 2013-02-23 00:40:36 PST
Created attachment 189910 [details]
Patch for landing
Comment 21 WebKit Review Bot 2013-02-23 02:51:40 PST
Comment on attachment 189910 [details]
Patch for landing

Clearing flags on attachment: 189910

Committed r143840: <http://trac.webkit.org/changeset/143840>
Comment 22 WebKit Review Bot 2013-02-23 02:51:44 PST
All reviewed patches have been landed.  Closing bug.
Comment 23 Vsevolod Vlasov 2013-02-25 04:43:13 PST
This patch caused assertion failures in multiple layout tests,

fullscreen/full-screen-enabled-prefixed.html
fast/dom/HTMLMeterElement/meter-element-repaint-on-update-value.html
fast/forms/month/month-stepup-stepdown.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fullscreen%2Ffull-screen-enabled-prefixed.html%2Cfast%2Fdom%2FHTMLMeterElement%2Fmeter-element-repaint-on-update-value.html%2Cfast%2Fforms%2Fmonth%2Fmonth-stepup-stepdown.html

crash log for DumpRenderTree (pid 2101):
STDOUT: <empty>
STDERR: ASSERTION FAILED: node->treeScope() == m_oldScope
STDERR: ../../third_party/WebKit/Source/WebCore/dom/TreeScopeAdopter.cpp(109) : void WebCore::TreeScopeAdopter::updateTreeScope(WebCore::Node *) const
STDERR: 1   0x74e2a2d WebCore::TreeScopeAdopter::updateTreeScope(WebCore::Node*) const
STDERR: 2   0x74e252a WebCore::TreeScopeAdopter::moveTreeToNewScope(WebCore::Node*) const
STDERR: 3   0x74e2644 WebCore::TreeScopeAdopter::moveTreeToNewScope(WebCore::Node*) const
STDERR: 4   0x74dd7e1 WebCore::TreeScopeAdopter::execute() const
STDERR: 5   0x74dc1f0 WebCore::TreeScope::adoptIfNeeded(WebCore::Node*)
STDERR: 6   0x727c569 WebCore::Private::NodeRemovalDispatcher<WebCore::Node, WebCore::ContainerNode, true>::dispatch(WebCore::Node*, WebCore::ContainerNode*)
STDERR: 7   0x727c4b4 void WebCore::Private::addChildNodesToDeletionQueue<WebCore::Node, WebCore::ContainerNode>(WebCore::Node*&, WebCore::Node*&, WebCore::ContainerNode*)
STDERR: 8   0x7278939 void WebCore::removeDetachedChildrenInContainer<WebCore::Node, WebCore::ContainerNode>(WebCore::ContainerNode*)
STDERR: 9   0x7272514 WebCore::ContainerNode::removeDetachedChildren()
STDERR: 10  0x74bdc6b WebCore::ShadowRoot::dispose()
STDERR: 11  0x74bdcc1 non-virtual thunk to WebCore::ShadowRoot::dispose()
STDERR: 12  0x7431179 WebCore::TreeScope::removedLastRefToScope()
STDERR: 13  0x74299fe WebCore::Node::removedLastRef()
STDERR: 14  0x6529bc4 WebCore::TreeShared<WebCore::Node>::deref()
STDERR: 15  0x7ba472d void WTF::derefIfNotNull<WebCore::ShadowRoot>(WebCore::ShadowRoot*)
STDERR: 16  0x7ba46bd WTF::RefPtr<WebCore::ShadowRoot>::~RefPtr()
STDERR: 17  0x7ba466b WTF::RefPtr<WebCore::ShadowRoot>::~RefPtr()
STDERR: 18  0x73804a4 WebCore::ElementShadow::removeAllShadowRoots()
STDERR: 19  0x73b858e WebCore::ElementShadow::~ElementShadow()
STDERR: 20  0x73b84cb WebCore::ElementShadow::~ElementShadow()
STDERR: 21  0x73b846d void WTF::deleteOwnedPtr<WebCore::ElementShadow>(WebCore::ElementShadow*)
STDERR: 22  0x73beda9 WTF::OwnPtr<WebCore::ElementShadow>::clear()
STDERR: 23  0x73bed45 WTF::OwnPtr<WebCore::ElementShadow>::operator=(nullptr_t)
STDERR: 24  0x7394f12 WebCore::ElementRareData::clearShadow()
STDERR: 25  0x7383409 WebCore::Element::~Element()
STDERR: 26  0x74c9652 WebCore::StyledElement::~StyledElement()
STDERR: 27  0x77e1e4b WebCore::HTMLElement::~HTMLElement()
STDERR: 28  0x765e4ab WebCore::LabelableElement::~LabelableElement()
STDERR: 29  0x76139ab WebCore::HTMLProgressElement::~HTMLProgressElement()
STDERR: 30  0x761395b WebCore::HTMLProgressElement::~HTMLProgressElement()
STDERR: 31  0x76138fe WebCore::HTMLProgressElement::~HTMLProgressElement()
STDERR: Received signal 11 SEGV_MAPERR 0000bbadbeef
STDERR:  [0x00000295287f]
STDERR:  [0x00000295281b]
STDERR:  [0x0000029524ab]
STDERR:  [0x000096f1059b]
STDERR:  [0x0000ffffffff]
STDERR:  [0x0000074e252a]
STDERR:  [0x0000074e2644]
STDERR:  [0x0000074dd7e1]
STDERR:  [0x0000074dc1f0]
STDERR:  [0x00000727c569]
STDERR:  [0x00000727c4b4]
STDERR:  [0x000007278939]
STDERR:  [0x000007272514]
STDERR:  [0x0000074bdc6b]
STDERR:  [0x0000074bdcc1]
STDERR:  [0x000007431179]
STDERR:  [0x0000074299fe]
STDERR:  [0x000006529bc4]
STDERR:  [0x000007ba472d]
STDERR:  [0x000007ba46bd]
STDERR:  [0x000007ba466b]
STDERR:  [0x0000073804a4]
STDERR:  [0x0000073b858e]
STDERR:  [0x0000073b84cb]
STDERR:  [0x0000073b846d]
STDERR:  [0x0000073beda9]
STDERR:  [0x0000073bed45]
STDERR:  [0x000007394f12]
STDERR:  [0x000007383409]
STDERR:  [0x0000074c9652]
STDERR:  [0x0000077e1e4b]
STDERR:  [0x00000765e4ab]
STDERR:  [0x0000076139ab]
STDERR:  [0x00000761395b]
STDERR:  [0x0000076138fe]
STDERR:  [0x000007429a20]
STDERR:  [0x000006529bc4]
STDERR:  [0x000007f29481]
STDERR:  [0x000008c57478]
STDERR:  [0x000007f6f857]
STDERR:  [0x0000030392a6]
STDERR:  [0x000003036802]
STDERR:  [0x000003051a1b]
STDERR:  [0x000003051229]
STDERR:  [0x000002f5a9ab]
STDERR:  [0x00000305213b]
STDERR:  [0x00000305201c]
STDERR:  [0x0000033c5fa6]
STDERR:  [0x0000033d0f70]
STDERR:  [0x000002f1d6ad]
STDERR:  [0x000002f0d8da]
STDERR:  [0x000002f0d7a9]
STDERR:  [0x000002eafdbb]
STDERR:  [0x000008ce3087]
STDERR:  [0x000008ce2408]
STDERR:  [0x000008c872f2]
STDERR:  [0x000008c86ff9]
STDERR:  [0x000008c8a535]
STDERR:  [0x00000962f9bb]
STDERR:  [0x000009485c06]
STDERR:  [0x00000945d00f]
STDERR:  [0x000006629bb5]
STDERR: ax: bbadbeef, bx: 7c214c01, cx: ab63af32, dx: ab63af32
STDERR: di: 9b30552, si: 9b304f6, bp: c0056878, sp: c0056840, ss: 23, flags: 10282
STDERR: ip: 74e2a37, cs: 1b, ds: 23, es: 23, fs: 0, gs: f
Comment 24 Vsevolod Vlasov 2013-02-25 05:04:27 PST
Filed https://bugs.webkit.org/show_bug.cgi?id=110749
Comment 25 Dimitri Glazkov (Google) 2013-02-25 10:13:38 PST
Rolled out in r143940.
Comment 26 Ryosuke Niwa 2013-02-25 11:47:26 PST
(In reply to comment #25)
> Rolled out in r143940.

I don't think rolling out the patch makes much sense. We're only seeing failures of an assertion added in this patch.

All these svn churns are ridiculously annoying.
Comment 27 Hajime Morrita 2013-03-04 20:04:25 PST
Created attachment 191385 [details]
Patch for landing
Comment 28 Hajime Morrita 2013-03-04 23:37:56 PST
Comment on attachment 191385 [details]
Patch for landing

The crash was due to Bug 108713. I included one line fix for that as well.
Comment 29 Elliott Sprehn 2013-03-04 23:46:25 PST
(In reply to comment #28)
> (From update of attachment 191385 [details])
> The crash was due to Bug 108713. I included one line fix for that as well.

What was the one line fix?
Comment 30 Hajime Morrita 2013-03-05 00:05:35 PST
(In reply to comment #29)
> (In reply to comment #28)
> > (From update of attachment 191385 [details] [details])
> > The crash was due to Bug 108713. I included one line fix for that as well.
> 
> What was the one line fix?

One copied from https://bugs.webkit.org/attachment.cgi?id=188241&action=review

--- a/Source/WebCore/dom/Element.cpp
+++ b/Source/WebCore/dom/Element.cpp
@@ -2753,6 +2753,7 @@ PassRefPtr<Attr> Element::ensureAttr(const QualifiedName& name)
     RefPtr<Attr> attrNode = findAttrNodeInList(attrNodeList, name);
     if (!attrNode) {
         attrNode = Attr::create(this, name);
+        treeScope()->adoptIfNeeded(attrNode.get());
         attrNodeList->append(attrNode);
     }
     return attrNode.release();
Comment 31 WebKit Review Bot 2013-03-05 00:10:19 PST
Comment on attachment 191385 [details]
Patch for landing

Clearing flags on attachment: 191385

Committed r144735: <http://trac.webkit.org/changeset/144735>
Comment 32 WebKit Review Bot 2013-03-05 00:10:26 PST
All reviewed patches have been landed.  Closing bug.
Comment 33 Alexey Proskuryakov 2013-12-16 15:52:19 PST
*** Bug 108710 has been marked as a duplicate of this bug. ***
Comment 34 Alexey Proskuryakov 2013-12-16 15:53:02 PST
*** Bug 108713 has been marked as a duplicate of this bug. ***