42610 2010-07-19 18:17:23 -0700 NotificationCenter::disconnectFrame will crash if called twice 2010-07-21 07:44:18 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Other OS X 10.5 RESOLVED DUPLICATE 42534 P2 Normal --- 1 johnnyg johnnyg ap kling webkit.review.bot yael oldest_to_newest 253502 0 johnnyg 2010-07-19 18:17:23 -0700 NotificationCenter::disconnectFrame will crash if called twice 253515 1 62024 johnnyg 2010-07-19 18:52:06 -0700 Created attachment 62024 Patch 253518 2 webkit.review.bot 2010-07-19 18:56:35 -0700 Attachment 62024 did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 253520 3 62026 johnnyg 2010-07-19 19:01:06 -0700 Created attachment 62026 Patch 253537 4 kling 2010-07-19 20:58:28 -0700 Related: 42534 253549 5 ap 2010-07-19 22:08:26 -0700 > Adds a null check; this is a prospective fix for a crash that is difficult to repro. Why is it difficult to reproduce? This looks like a case where having a regression test is particularly important. > NotificationCenter::disconnectFrame will crash if called twice How can it be called twice? 253787 6 johnnyg 2010-07-20 10:12:11 -0700 (In reply to comment #5) > > Adds a null check; this is a prospective fix for a crash that is difficult to repro. > > Why is it difficult to reproduce? This looks like a case where having a regression test is particularly important. > The crash is being reported by Chrome users, with a stack trace that suggests this code path, and the timing suggests http://trac.webkit.org/changeset/62939 may be the culprit. When I say difficult to repro, actually I have not been able to reproduce it at all, but because of the crash reports, we know it is happening. > > NotificationCenter::disconnectFrame will crash if called twice > > How can it be called twice? Again, not sure that's what's happening. But if it did happen, it will definitely crash. But as Andreas pointed out, bug 42534 has more information. 253820 7 ap 2010-07-20 11:07:19 -0700 There are multiple disconnectFrame() calls in various objects, all called from DOMWindow::clear(), and those don't seem to cause crashes. One way to attack this is to answer the question of what is different about NotificationCenter. 253822 8 johnnyg 2010-07-20 11:09:07 -0700 I think Yael understands the cause of the crash in bug 42534, so I will withdraw this patch for now. 253909 9 yael 2010-07-20 14:16:20 -0700 Not being able to reproduce this is slowing me down, and if someone can find a way to reproduce, I will be happy to know about it. I am working on this, and sorry for the inconvenience. 254082 10 yael 2010-07-20 17:44:26 -0700 (In reply to comment #7) > One way to attack this is to answer the question of what is different about NotificationCenter. This is a very good question :-) Since notifications are expected to outlive the page that created them, I made Qt's NotificationPresenter a singleton. It is deleted when the last page is deleted. John, is it a singleton in Chromium too? 254273 11 yael 2010-07-21 07:44:18 -0700 *** This bug has been marked as a duplicate of bug 42534 *** 62024 2010-07-19 18:52:06 -0700 2010-07-19 19:01:03 -0700 Patch bug-42610-20100719185205.patch text/plain 1256 johnnyg SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MzcwNCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDctMTkgIEpvaG4gR3JlZ2cgIDxqb2hubnlnQGdvb2dsZS5j b20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm90 aWZpY2F0aW9uQ2VudGVyOjpkaXNjb25uZWN0RnJhbWUgd2lsbCBjcmFzaCBpZiBjYWxsZWQgdHdp Y2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQyNjEw CisKKyAgICAgICAgQWRkcyBhIG51bGwgY2hlY2s7IHRoaXMgaXMgYSBwcm9zcGVjdGl2ZSBmaXgg Zm9yIGEgY3Jhc2ggdGhhdCBpcyBkaWZmaWN1bHQgdG8gcmVwcm8uCisJCisgICAgICAgICogbm90 aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25DZW50ZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Tm90 aWZpY2F0aW9uQ2VudGVyOjpkaXNjb25uZWN0RnJhbWUpOgorCiAyMDEwLTA3LTE5ICBBbmRlcnMg Q2FybHNzb24gIDxhbmRlcnNjYUBhcHBsZS5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRGFy aW4gQWRsZXIsIEFkYW0gUm9iZW4sIERhbiBCZXJuc3RlaW4gYW5kIFNhbSBXZWluaWcuCkluZGV4 OiBXZWJDb3JlL25vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9uQ2VudGVyLmNwcAo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBXZWJDb3JlL25vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9uQ2VudGVyLmNwcAkocmV2aXNp b24gNjM2MDkpCisrKyBXZWJDb3JlL25vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9uQ2VudGVyLmNw cAkod29ya2luZyBjb3B5KQpAQCAtNjEsNiArNjEsOCBAQCB2b2lkIE5vdGlmaWNhdGlvbkNlbnRl cjo6cmVxdWVzdFBlcm1pc3NpCiAKIHZvaWQgTm90aWZpY2F0aW9uQ2VudGVyOjpkaXNjb25uZWN0 RnJhbWUoKQogeworICAgIGlmICghcHJlc2VudGVyKCkpCisgICAgICAgIHJldHVybjsKICAgICBt X25vdGlmaWNhdGlvblByZXNlbnRlci0+Y2FuY2VsUmVxdWVzdHNGb3JQZXJtaXNzaW9uKG1fc2Ny aXB0RXhlY3V0aW9uQ29udGV4dCk7CiAgICAgbV9ub3RpZmljYXRpb25QcmVzZW50ZXIgPSAwOwog fQo= 62026 2010-07-19 19:01:06 -0700 2010-07-20 11:08:25 -0700 Patch bug-42610-20100719190104.patch text/plain 1255 johnnyg SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MzcwNCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDctMTkgIEpvaG4gR3JlZ2cgIDxqb2hubnlnQGdvb2dsZS5j b20+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm90 aWZpY2F0aW9uQ2VudGVyOjpkaXNjb25uZWN0RnJhbWUgd2lsbCBjcmFzaCBpZiBjYWxsZWQgdHdp Y2UKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQyNjEw CisKKyAgICAgICAgQWRkcyBhIG51bGwgY2hlY2s7IHRoaXMgaXMgYSBwcm9zcGVjdGl2ZSBmaXgg Zm9yIGEgY3Jhc2ggdGhhdCBpcyBkaWZmaWN1bHQgdG8gcmVwcm8uCisKKyAgICAgICAgKiBub3Rp ZmljYXRpb25zL05vdGlmaWNhdGlvbkNlbnRlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpOb3Rp ZmljYXRpb25DZW50ZXI6OmRpc2Nvbm5lY3RGcmFtZSk6CisKIDIwMTAtMDctMTkgIEFuZGVycyBD YXJsc3NvbiAgPGFuZGVyc2NhQGFwcGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXJp biBBZGxlciwgQWRhbSBSb2JlbiwgRGFuIEJlcm5zdGVpbiBhbmQgU2FtIFdlaW5pZy4KSW5kZXg6 IFdlYkNvcmUvbm90aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25DZW50ZXIuY3BwCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFdlYkNvcmUvbm90aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25DZW50ZXIuY3BwCShyZXZpc2lv biA2MzYwOSkKKysrIFdlYkNvcmUvbm90aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25DZW50ZXIuY3Bw CSh3b3JraW5nIGNvcHkpCkBAIC02MSw2ICs2MSw4IEBAIHZvaWQgTm90aWZpY2F0aW9uQ2VudGVy OjpyZXF1ZXN0UGVybWlzc2kKIAogdm9pZCBOb3RpZmljYXRpb25DZW50ZXI6OmRpc2Nvbm5lY3RG cmFtZSgpCiB7CisgICAgaWYgKCFwcmVzZW50ZXIoKSkKKyAgICAgICAgcmV0dXJuOwogICAgIG1f bm90aWZpY2F0aW9uUHJlc2VudGVyLT5jYW5jZWxSZXF1ZXN0c0ZvclBlcm1pc3Npb24obV9zY3Jp cHRFeGVjdXRpb25Db250ZXh0KTsKICAgICBtX25vdGlmaWNhdGlvblByZXNlbnRlciA9IDA7CiB9 Cg==