Summary: | Replace documentFragmentIsShadowRoot with isTreeScope | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Elliott Sprehn <esprehn> | ||||||||||||||
Component: | New Bugs | Assignee: | Elliott Sprehn <esprehn> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | dglazkov, loislo, morrita, ojan.autocc, philn, roger_fong, thorton, webkit.review.bot, xan.lopez | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 105621 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Elliott Sprehn
2012-12-18 13:55:26 PST
Created attachment 180022 [details]
Patch
Created attachment 180030 [details]
Patch
Comment on attachment 180030 [details] Patch Attachment 180030 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15413296 (In reply to comment #3) > (From update of attachment 180030 [details]) > Attachment 180030 [details] did not pass win-ews (win): > Output: http://queues.webkit.org/results/15413296 Hmm, gtk says: ls.a(libWebCoreInternals_la-Internals.o):Internals.cpp:function WebCore::Internals::numberOfScopedHTMLStyleChildren(WebCore::Node const*, int&) const: error: undefined reference to 'WebCore::Node::isTreeScope() const' and win-ews says: 16>WebCoreTestSupport.lib(Internals.obj) : error LNK2019: unresolved external symbol "public: bool __thiscall WebCore::Node::isTreeScope(void)const " (?isTreeScope@Node@WebCore@@QBE_NXZ) referenced in function "public: bool __thiscall WebCore::Node::isShadowRoot(void)const " (?isShadowRoot@Node@WebCore@@QBE_NXZ) 16>C:\cygwin\home\buildbot\WebKit\WebKitBuild\Debug\bin\DumpRenderTree.dll : fatal error LNK1120: 1 unresolved externals Looks like we need to export isTreeScope which wasn't needed for documentFragmentIsShadowRoot since it was virtual? I'll upload again with a WebCore.exp.in change. Comment on attachment 180030 [details] Patch Attachment 180030 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15399493 Created attachment 180432 [details]
Patch
Comment on attachment 180432 [details] Patch Attachment 180432 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/15460128 Created attachment 180445 [details]
Patch
Comment on attachment 180445 [details]
Patch
You can also just do webkit-patch land-safely. You're a committer now, right?
(In reply to comment #9) > (From update of attachment 180445 [details]) > You can also just do webkit-patch land-safely. You're a committer now, right? Yeah, I realized I had cq+'ed before the gtk build had gone green so I removed it to wait to see if my exports file changes would work. :) Comment on attachment 180445 [details] Patch Clearing flags on attachment: 180445 Committed r138338: <http://trac.webkit.org/changeset/138338> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 105621 This also broke the Apple Windows port build for some unknown reason. (In reply to comment #14) > This also broke the Apple Windows port build for some unknown reason. STDOUT: <empty> STDERR: ASSERTION FAILED: scope STDERR: /Volumes/data/b/build/slave/webkit-mac-latest-dbg/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../html/HTMLStyleElement.cpp(160) : void WebCore::HTMLStyleElement::unregisterWithScopingNode(WebCore::ContainerNode *) STDERR: 1 0x1d74190 WebCore::HTMLStyleElement::unregisterWithScopingNode(WebCore::ContainerNode*) STDERR: 2 0x1d748d3 WebCore::HTMLStyleElement::removedFrom(WebCore::ContainerNode*) STDERR: 3 0x180dedf WebCore::ChildNodeRemovalNotifier::notifyNodeRemovedFromDocument(WebCore::Node*) STDERR: 4 0x18107dc WebCore::ChildNodeRemovalNotifier::notifyDescendantRemovedFromDocument(WebCore::ContainerNode*) STDERR: 5 0x180df11 WebCore::ChildNodeRemovalNotifier::notifyNodeRemovedFromDocument(WebCore::Node*) STDERR: 6 0x180a967 WebCore::ChildNodeRemovalNotifier::notify(WebCore::Node*) STDERR: 7 0x18f2d0d WebCore::ElementShadow::removeAllShadowRoots() STDERR: 8 0x18f7db4 WebCore::Element::~Element() STDERR: 9 0x1a2c022 WebCore::StyledElement::~StyledElement() STDERR: 10 0x1d1f79b WebCore::HTMLElement::~HTMLElement() STDERR: 11 0x1f02d0b WebCore::HTMLDivElement::~HTMLDivElement() STDERR: 12 0x1cac25b WebCore::HTMLDivElement::~HTMLDivElement() STDERR: 13 0x1cac2ae WebCore::HTMLDivElement::~HTMLDivElement() STDERR: 14 0x180a0dd void WebCore::removeAllChildrenInContainer<WebCore::Node, WebCore::ContainerNode>(WebCore::ContainerNode*) STDERR: 15 0x180411b WebCore::ContainerNode::removeAllChildren() STDERR: 16 0x182a26d WebCore::Document::removedLastRef() STDERR: 17 0x1987ab6 WebCore::Node::removedLastRef() STDERR: 18 0x759808 WebCore::TreeShared<WebCore::Node, WebCore::ContainerNode>::deref() STDERR: 19 0xcb1361 WebCore::V8HTMLDocument::derefObject(void*) STDERR: 20 0x25bf168 WebCore::WrapperTypeInfo::derefObject(void*) STDERR: 21 0x25be7a9 WebCore::DOMDataStore::weakCallback(v8::Persistent<v8::Value>, void*) STDERR: 22 0x8b47881 v8::internal::GlobalHandles::Node::PostGarbageCollectionProcessing(v8::internal::Isolate*, v8::internal::GlobalHandles*) STDERR: 23 0x8b45812 v8::internal::GlobalHandles::PostGarbageCollectionProcessing(v8::internal::GarbageCollector, v8::internal::GCTracer*) STDERR: 24 0x8b5e10f v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::internal::GCTracer*) STDERR: 25 0x8b5d839 v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollector, char const*, char const*) STDERR: 26 0x8a7444b v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, char const*) STDERR: 27 0x8b5e71b _ZN2v88internalL40AbortIncrementalMarkingAndCollectGarbageEPNS0_4HeapENS0_15AllocationSpaceEPKc STDERR: 28 0x8b5e5fc v8::internal::Heap::ReserveSpace(int*, unsigned char**) STDERR: 29 0x8eb3246 v8::internal::Deserializer::DeserializePartial(v8::internal::Object**) STDERR: 30 0x8ebe0e0 v8::internal::Snapshot::NewContextFromSnapshot() STDERR: 31 0x8a3ff9d v8::internal::Genesis::Genesis(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::Handle<v8::ObjectTemplate>, v8::ExtensionConfiguration*) STDERR: Received signal 11 (In reply to comment #15) > (In reply to comment #14) > > This also broke the Apple Windows port build for some unknown reason. > > STDOUT: <empty> > STDERR: ASSERTION FAILED: scope > STDERR: /Volumes/data/b/build/slave/webkit-mac-latest-dbg/build/src/third_party/WebKit/Source/WebCore/WebCore.gyp/../html/HTMLStyleElement.cpp(160) : void WebCore::HTMLStyleElement::unregisterWithScopingNode(WebCore::ContainerNode *) > STDERR: 1 0x1d74190 Woops, this is because of the bug noticed a few weeks ago where we break the tree scope of shadow roots during removal by adopting them into the document. While usually shadowRoot->treeScope() == shadowRoot, inside of removeAllShadowRoots() we document()->adoptIfNeeded(shadowRoot) which makes shadowRoot->treeScope() == document() instead. I think I have a fix, running all the tests now. Created attachment 180538 [details]
Patch
(In reply to comment #17) > Created an attachment (id=180538) [details] > Patch I ran all the tests and the crashes went away. (In reply to comment #18) > (In reply to comment #17) > > Created an attachment (id=180538) [details] [details] > > Patch > > I ran all the tests and the crashes went away. Staring at this for longer we should have oldRoot->setParentTreeScope(shadowHost->document()); where the adopt used to be. It seems we always had a bug where the parent tree scope ptr could have been freed, but I can't figure out a way to exploit that since you'd need to make us go through related target adjustment on a ShadowRoot that's had it's host collected. I'll add that line and upload with cq+. Created attachment 180565 [details]
Patch for landing
Comment on attachment 180565 [details] Patch for landing Clearing flags on attachment: 180565 Committed r138404: <http://trac.webkit.org/changeset/138404> All reviewed patches have been landed. Closing bug. |