136333 2014-08-27 23:28:33 -0700 AX: Safari at com.apple.WebCore: WebCore::AXObjectCache::clearTextMarkerNodesInUse 2014-08-28 11:01:15 -0700 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 cfleizach cfleizach aboxhall apinheiro commit-queue dmazzoni jcraig jdiggs mario samuel_white webkit-bug-importer oldest_to_newest 1031920 0 cfleizach 2014-08-27 23:28:33 -0700 (lldb) bt * thread #1: tid = 0x12d68, 0x000000010adae22a JavaScriptCore`WTFCrash + 42 at Assertions.cpp:329, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef) * frame #0: 0x000000010adae22a JavaScriptCore`WTFCrash + 42 at Assertions.cpp:329 frame #1: 0x000000010eb5f586 WebCore`WebCore::Node::treeScope(this=0x00007f9e4afcded0) const + 70 at Node.h:401 frame #2: 0x000000010eb5a603 WebCore`WebCore::Node::document(this=0x00007f9e4afcded0) const + 83 at Node.h:396 frame #3: 0x000000010eb2ebb5 WebCore`WebCore::AXObjectCache::clearTextMarkerNodesInUse(this=0x00007f9e3bc20b30, document=0x0000000000000000) + 149 at AXObjectCache.cpp:1051 frame #4: 0x000000010f3f2fa2 WebCore`WebCore::Frame::disconnectOwnerElement(this=0x00007f9e4b7af2f0) + 114 at Frame.cpp:814 frame #5: 0x000000010f3f2745 WebCore`WebCore::Frame::~Frame(this=0x00007f9e4b7af2f0) + 149 at Frame.cpp:224 frame #6: 0x000000010f3f3005 WebCore`WebCore::Frame::~Frame(this=0x00007f9e4b7af2f0) + 21 at Frame.cpp:214 frame #7: 0x000000010f3f3029 WebCore`WebCore::Frame::~Frame(this=0x00007f9e4b7af2f0) + 25 at Frame.cpp:214 frame #8: 0x000000010ebbd1f3 WebCore`WTF::RefCounted<WebCore::Frame>::deref(this=0x00007f9e4b7af2f8) + 83 at RefCounted.h:146 frame #9: 0x000000010ebbd18f WebCore`WTF::Ref<WebCore::Frame>::~Ref(this=0x00007fff556c6e58) + 31 at Ref.h:41 frame #10: 0x000000010ebb5fc5 WebCore`WTF::Ref<WebCore::Frame>::~Ref(this=0x00007fff556c6e58) + 21 at Ref.h:41 frame #11: 0x000000010f417e1f WebCore`WTF::VectorDestructor<true, WTF::Ref<WebCore::Frame> >::destruct(begin=0x00007fff556c6e58, end=0x00007fff556c6e70) + 47 at Vector.h:56 frame #12: 0x000000010f417ddd WebCore`WTF::VectorTypeOperations<WTF::Ref<WebCore::Frame> >::destruct(begin=0x00007fff556c6e58, end=0x00007fff556c6e70) + 29 at Vector.h:220 frame #13: 0x000000010f417d50 WebCore`WTF::Vector<WTF::Ref<WebCore::Frame>, 16ul, WTF::CrashOnOverflow>::shrink(this=0x00007fff556c6e48, size=0) + 128 at Vector.h:957 frame #14: 0x000000010f417cb4 WebCore`WTF::Vector<WTF::Ref<WebCore::Frame>, 16ul, WTF::CrashOnOverflow>::~Vector(this=0x00007fff556c6e48) + 52 at Vector.h:596 frame #15: 0x000000010f416535 WebCore`WTF::Vector<WTF::Ref<WebCore::Frame>, 16ul, WTF::CrashOnOverflow>::~Vector(this=0x00007fff556c6e48) + 21 at Vector.h:594 frame #16: 0x000000010f402055 WebCore`WebCore::FrameLoader::detachChildren(this=0x00007f9e435b0ff0) + 309 at FrameLoader.cpp:2415 <rdar://problem/17030054> 1031925 1 237294 cfleizach 2014-08-27 23:33:16 -0700 Created attachment 237294 patch 1031955 2 237294 ddkilzer 2014-08-28 04:10:10 -0700 Comment on attachment 237294 patch View in context: https://bugs.webkit.org/attachment.cgi?id=237294&action=review r=me but please consider the comment below. > Source/WebCore/accessibility/AXObjectCache.cpp:1051 > + if (node->inDocument() && &(node)->document() == document) Is there a reason to keep nodes not in a document in m_textMarkerNodes? Just wondering if this condition should be || instead of && (and negated) since we might build up document-less nodes in the cache over time with the !document early return above: If (!node->inDocument() || &node->document() == document) 1032030 3 237294 cfleizach 2014-08-28 10:51:21 -0700 Comment on attachment 237294 patch View in context: https://bugs.webkit.org/attachment.cgi?id=237294&action=review >> Source/WebCore/accessibility/AXObjectCache.cpp:1051 >> + if (node->inDocument() && &(node)->document() == document) > > Is there a reason to keep nodes not in a document in m_textMarkerNodes? Just wondering if this condition should be || instead of && (and negated) since we might build up document-less nodes in the cache over time with the !document early return above: > > If (!node->inDocument() || &node->document() == document) I think your logic makes sense here. I'm gonna go in that direction 1032033 4 cfleizach 2014-08-28 11:01:15 -0700 http://trac.webkit.org/changeset/173067 237294 2014-08-27 23:33:16 -0700 2014-08-28 10:51:21 -0700 patch patch text/plain 2120 cfleizach SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3MzA1MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDE0LTA4LTI3ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IFNhZmFyaSBhdCBj b20uYXBwbGUuV2ViQ29yZTogV2ViQ29yZTo6QVhPYmplY3RDYWNoZTo6Y2xlYXJUZXh0TWFya2Vy Tm9kZXNJblVzZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTM2MzMzCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgSWYgYSBOb2RlIGlzIGFza2VkIGZvciBpdCdzIERvY3VtZW50IHdoZW4gaXQncyBub3QgYWN0 dWFsbHkgaW4gYSBkb2N1bWVudCwgaXQgY2FuIGxlYWQgdG8gYW4gYXNzZXJ0L2NyYXNoLgorICAg ICAgICBXZSBjYW4gYXZvaWQgdGhpcyBieSBjaGVja2luZyB0aGF0IHRoZSBub2RlIGlzIGluIGEg ZG9jdW1lbnQgYmVmb3JlIGFza2luZyBmb3IgaXRzIGRvY3VtZW50LgorCisgICAgICAgIEkgd2Fz IG5vdCBhYmxlIHRvIG1ha2UgYSB0ZXN0IGNhc2UuICAgICAgCisKKyAgICAgICAgKiBhY2Nlc3Np YmlsaXR5L0FYT2JqZWN0Q2FjaGUuY3BwOgorICAgICAgICAoV2ViQ29yZTo6QVhPYmplY3RDYWNo ZTo6Y2xlYXJUZXh0TWFya2VyTm9kZXNJblVzZSk6CisKIDIwMTQtMDgtMjcgIENzYWJhIE9zenRy b2dvbsOhYyAgPG9zc3lAd2Via2l0Lm9yZz4KIAogICAgICAgICBPbmUgbW9yZSBVUlRCRiBhZnRl ciByMTczMDQ3LgpJbmRleDogU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9BWE9iamVjdENh Y2hlLmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5L0FYT2Jq ZWN0Q2FjaGUuY3BwCShyZXZpc2lvbiAxNzMwMzQpCisrKyBTb3VyY2UvV2ViQ29yZS9hY2Nlc3Np YmlsaXR5L0FYT2JqZWN0Q2FjaGUuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xMDQyLDIwICsxMDQy LDE4IEBACiAKIHZvaWQgQVhPYmplY3RDYWNoZTo6Y2xlYXJUZXh0TWFya2VyTm9kZXNJblVzZShE b2N1bWVudCogZG9jdW1lbnQpCiB7Ci0gICAgSGFzaFNldDxOb2RlKj46Oml0ZXJhdG9yIGl0ID0g bV90ZXh0TWFya2VyTm9kZXMuYmVnaW4oKTsKLSAgICBIYXNoU2V0PE5vZGUqPjo6aXRlcmF0b3Ig ZW5kID0gbV90ZXh0TWFya2VyTm9kZXMuZW5kKCk7Ci0KKyAgICBpZiAoIWRvY3VtZW50KQorICAg ICAgICByZXR1cm47CisgICAgCiAgICAgLy8gQ2hlY2sgZWFjaCBub2RlIHRvIHNlZSBpZiBpdCdz IGluc2lkZSB0aGUgZG9jdW1lbnQgYmVpbmcgZGVsZXRlZC4KICAgICBIYXNoU2V0PE5vZGUqPiBu b2Rlc1RvRGVsZXRlOwotICAgIGZvciAoOyBpdCAhPSBlbmQ7ICsraXQpIHsKLSAgICAgICAgaWYg KCYoKml0KS0+ZG9jdW1lbnQoKSA9PSBkb2N1bWVudCkKLSAgICAgICAgICAgIG5vZGVzVG9EZWxl dGUuYWRkKCppdCk7CisgICAgZm9yIChjb25zdCBhdXRvJiBub2RlIDogbV90ZXh0TWFya2VyTm9k ZXMpIHsKKyAgICAgICAgaWYgKG5vZGUtPmluRG9jdW1lbnQoKSAmJiAmKG5vZGUpLT5kb2N1bWVu dCgpID09IGRvY3VtZW50KQorICAgICAgICAgICAgbm9kZXNUb0RlbGV0ZS5hZGQobm9kZSk7CiAg ICAgfQogICAgIAotICAgIGl0ID0gbm9kZXNUb0RlbGV0ZS5iZWdpbigpOwotICAgIGVuZCA9IG5v ZGVzVG9EZWxldGUuZW5kKCk7Ci0gICAgZm9yICg7IGl0ICE9IGVuZDsgKytpdCkKLSAgICAgICAg bV90ZXh0TWFya2VyTm9kZXMucmVtb3ZlKCppdCk7CisgICAgZm9yIChjb25zdCBhdXRvJiBub2Rl IDogbm9kZXNUb0RlbGV0ZSkKKyAgICAgICAgbV90ZXh0TWFya2VyTm9kZXMucmVtb3ZlKG5vZGUp OwogfQogICAgIAogYm9vbCBBWE9iamVjdENhY2hlOjpub2RlSXNUZXh0Q29udHJvbChjb25zdCBO b2RlKiBub2RlKQo=