39998 2010-06-01 07:53:55 -0700 Notification object ref counting is not correct. 2010-07-30 04:12:38 -0700 1 1 1 Unclassified WebKit WebCore JavaScript 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 40051 39995 1 yael webkit-unassigned abarth commit-queue eric hausmann johnnyg ossy webkit.review.bot oldest_to_newest 232748 0 yael 2010-06-01 07:53:55 -0700 JSNotification objects are marked for garbage collection right after being created and are deleted shortly after, even if they have event listeners. A patch is coming soon. 232779 1 57551 yael 2010-06-01 09:19:31 -0700 Created attachment 57551 Patch, fix the refcounting. 232781 2 johnnyg 2010-06-01 09:22:42 -0700 I'm not a reviewer, but looks good to me -- that's the same fix I've been testing locally. 232933 3 57551 darin 2010-06-01 13:33:00 -0700 Comment on attachment 57551 Patch, fix the refcounting. > - Notification* createHTMLNotification(const String& URI, ExceptionCode& ec) > + PassRefPtr<Notification> createHTMLNotification(const String& URI, ExceptionCode& ec) This change is great. > - return Notification::create(m_scriptExecutionContext->completeURL(URI), context(), ec, presenter()); > + return adoptRef(Notification::create(m_scriptExecutionContext->completeURL(URI), context(), ec, presenter())); This seems wrong. The adoptRef should be inside Notification::create and it should return a PassRefPtr! 232964 4 57599 yael 2010-06-01 15:02:57 -0700 Created attachment 57599 Move the adoptRef to Notification::create() 233137 5 57599 commit-queue 2010-06-02 01:45:56 -0700 Comment on attachment 57599 Move the adoptRef to Notification::create() Clearing flags on attachment: 57599 Committed r60547: <http://trac.webkit.org/changeset/60547> 233138 6 commit-queue 2010-06-02 01:46:02 -0700 All reviewed patches have been landed. Closing bug. 233149 7 webkit.review.bot 2010-06-02 02:19:01 -0700 http://trac.webkit.org/changeset/60547 might have broken Qt Linux Release The following changes are on the blame list: http://trac.webkit.org/changeset/60544 http://trac.webkit.org/changeset/60545 http://trac.webkit.org/changeset/60546 http://trac.webkit.org/changeset/60547 http://trac.webkit.org/changeset/60543 233190 8 ossy 2010-06-02 03:36:58 -0700 It was rolled out by http://trac.webkit.org/changeset/60554 , because it made fast/overflow/overflow-with-local-background-attachment.html crash. I tried to reproduce the crash manually. If I ran only this test or all fast/overflow tests, the crash didn't occured, but with all fast tests. Unfortunately I can't reproduce it in debug mode. :( 233210 9 ossy 2010-06-02 05:06:35 -0700 Reopened because the patch rolled-out. 233217 10 yael 2010-06-02 05:29:50 -0700 (In reply to comment #9) > Reopened because the patch rolled-out. Looking at the build results from last night, that test started crashing in 60543,and my patch was 60547. I am going to try the commit queue again. 233220 11 ossy 2010-06-02 05:34:44 -0700 (In reply to comment #10) > Looking at the build results from last night, that test started crashing in 60543,and my patch was 60547. No. Just check http://build.webkit.org/waterfall?show=Qt%20Linux%20Release 60543 - 60547 were on blame list, because 60542 was built in Build 12683 and 60547 was built in Build 12684. And then I tested it manually and 60547 was the culprit (Build 12690) 233237 12 yael 2010-06-02 06:33:54 -0700 (In reply to comment #11) > (In reply to comment #10) > > Looking at the build results from last night, that test started crashing in 60543,and my patch was 60547. > No. Just check http://build.webkit.org/waterfall?show=Qt%20Linux%20Release > 60543 - 60547 were on blame list, because 60542 was built in Build 12683 and 60547 was built in Build 12684. And then I tested it manually and 60547 was the culprit (Build 12690) Yes, you are right. However, I just verified that if I apply 40003 and then 39998, the crash does not happen anymore. I was afraid of temporary regression due to me splitting the notification work into smaller patches, and I guess this is what happened :-) 233314 13 yael 2010-06-02 10:17:33 -0700 Recommitted as 60569. After 60566, I could not reproduce the crash that caused this patch to be rolled out before. 258130 14 hausmann 2010-07-30 04:12:38 -0700 Revision r60547 cherry-picked into qtwebkit-2.1 with commit 9c33399bef9921ae60fe5d6ee573b9e7569eef84 57551 2010-06-01 09:19:31 -0700 2010-06-01 15:02:57 -0700 Patch, fix the refcounting. 39998.patch text/plain 2807 yael SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MDQ3OCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjAgQEAKKzIwMTAtMDYtMDEgIFlhZWwgQWhhcm9uICA8eWFlbC5haGFyb25Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE5vdGlmaWNhdGlvbiBvYmplY3QgcmVmIGNvdW50aW5nIGlzIG5vdCBjb3JyZWN0LgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzk5OTgKKworICAgICAg ICBSZXR1cm4gUGFzc1JlZlB0cjxOb3RpZmljYXRpb24+IGluc3RlYWQgb2YgYSByYXcgcG9pbnRl ciBmcm9tIHRoZSBjcmVhdGUgbWV0aG9kcworICAgICAgICBmb3IgV2ViIE5vdGlmaWNhdGlvbnMu IAorCisgICAgICAgIE5vIG5ldyB0ZXN0cyBhcmUgbmVlZGVkLCBhcyBleGlzdGluZyB0ZXN0cyBz aG93IHRoZSBwcm9ibGVtIHdoZW4gc2VuZGluZyB0aGUgCisgICAgICAgICJkaXNwbGF5IiBldmVu dCBhc3luY2hyb25vdXNseS4KKworICAgICAgICAqIG5vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9u Q2VudGVyLmg6CisgICAgICAgIChXZWJDb3JlOjpOb3RpZmljYXRpb25DZW50ZXI6OmNyZWF0ZUhU TUxOb3RpZmljYXRpb24pOgorICAgICAgICAoV2ViQ29yZTo6Tm90aWZpY2F0aW9uQ2VudGVyOjpj cmVhdGVOb3RpZmljYXRpb24pOgorCiAyMDEwLTA2LTAxICBBZGFtIExhbmdsZXkgIDxhZ2xAY2hy b21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEVyaWMgU2VpZGVsLgpJbmRleDogV2Vi Q29yZS9ub3RpZmljYXRpb25zL05vdGlmaWNhdGlvbkNlbnRlci5oCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdl YkNvcmUvbm90aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25DZW50ZXIuaAkocmV2aXNpb24gNjA0Nzgp CisrKyBXZWJDb3JlL25vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9uQ2VudGVyLmgJKHdvcmtpbmcg Y29weSkKQEAgLTQ5LDcgKzQ5LDcgQEAgbmFtZXNwYWNlIFdlYkNvcmUgewogICAgIHB1YmxpYzoK ICAgICAgICAgc3RhdGljIFBhc3NSZWZQdHI8Tm90aWZpY2F0aW9uQ2VudGVyPiBjcmVhdGUoU2Ny aXB0RXhlY3V0aW9uQ29udGV4dCogY29udGV4dCwgTm90aWZpY2F0aW9uUHJlc2VudGVyKiBwcmVz ZW50ZXIpIHsgcmV0dXJuIGFkb3B0UmVmKG5ldyBOb3RpZmljYXRpb25DZW50ZXIoY29udGV4dCwg cHJlc2VudGVyKSk7IH0KIAotICAgICAgICBOb3RpZmljYXRpb24qIGNyZWF0ZUhUTUxOb3RpZmlj YXRpb24oY29uc3QgU3RyaW5nJiBVUkksIEV4Y2VwdGlvbkNvZGUmIGVjKQorICAgICAgICBQYXNz UmVmUHRyPE5vdGlmaWNhdGlvbj4gY3JlYXRlSFRNTE5vdGlmaWNhdGlvbihjb25zdCBTdHJpbmcm IFVSSSwgRXhjZXB0aW9uQ29kZSYgZWMpCiAgICAgICAgIHsKICAgICAgICAgICAgIGlmICghcHJl c2VudGVyKCkpIHsKICAgICAgICAgICAgICAgICBlYyA9IElOVkFMSURfU1RBVEVfRVJSOwpAQCAt NTksMTcgKzU5LDE3IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKICAgICAgICAgICAgICAgICBlYyA9 IFNZTlRBWF9FUlI7CiAgICAgICAgICAgICAgICAgcmV0dXJuIDA7CiAgICAgICAgICAgICB9Ci0g ICAgICAgICAgICByZXR1cm4gTm90aWZpY2F0aW9uOjpjcmVhdGUobV9zY3JpcHRFeGVjdXRpb25D b250ZXh0LT5jb21wbGV0ZVVSTChVUkkpLCBjb250ZXh0KCksIGVjLCBwcmVzZW50ZXIoKSk7Cisg ICAgICAgICAgICByZXR1cm4gYWRvcHRSZWYoTm90aWZpY2F0aW9uOjpjcmVhdGUobV9zY3JpcHRF eGVjdXRpb25Db250ZXh0LT5jb21wbGV0ZVVSTChVUkkpLCBjb250ZXh0KCksIGVjLCBwcmVzZW50 ZXIoKSkpOwogICAgICAgICB9CiAKLSAgICAgICAgTm90aWZpY2F0aW9uKiBjcmVhdGVOb3RpZmlj YXRpb24oY29uc3QgU3RyaW5nJiBpY29uVVJJLCBjb25zdCBTdHJpbmcmIHRpdGxlLCBjb25zdCBT dHJpbmcmIGJvZHksIEV4Y2VwdGlvbkNvZGUmIGVjKQorICAgICAgICBQYXNzUmVmUHRyPE5vdGlm aWNhdGlvbj4gY3JlYXRlTm90aWZpY2F0aW9uKGNvbnN0IFN0cmluZyYgaWNvblVSSSwgY29uc3Qg U3RyaW5nJiB0aXRsZSwgY29uc3QgU3RyaW5nJiBib2R5LCBFeGNlcHRpb25Db2RlJiBlYykKICAg ICAgICAgewogICAgICAgICAgICAgaWYgKCFwcmVzZW50ZXIoKSkgewogICAgICAgICAgICAgICAg IGVjID0gSU5WQUxJRF9TVEFURV9FUlI7CiAgICAgICAgICAgICAgICAgcmV0dXJuIDA7CiAgICAg ICAgICAgICB9CiAgICAgICAgICAgICBOb3RpZmljYXRpb25Db250ZW50cyBjb250ZW50cyhpY29u VVJJLmlzRW1wdHkoKSA/IEtVUkwoKSA6IG1fc2NyaXB0RXhlY3V0aW9uQ29udGV4dC0+Y29tcGxl dGVVUkwoaWNvblVSSSksIHRpdGxlLCBib2R5KTsKLSAgICAgICAgICAgIHJldHVybiBOb3RpZmlj YXRpb246OmNyZWF0ZShjb250ZW50cywgY29udGV4dCgpLCBlYywgcHJlc2VudGVyKCkpOworICAg ICAgICAgICAgcmV0dXJuIGFkb3B0UmVmKE5vdGlmaWNhdGlvbjo6Y3JlYXRlKGNvbnRlbnRzLCBj b250ZXh0KCksIGVjLCBwcmVzZW50ZXIoKSkpOwogICAgICAgICB9CiAKICAgICAgICAgU2NyaXB0 RXhlY3V0aW9uQ29udGV4dCogY29udGV4dCgpIGNvbnN0IHsgcmV0dXJuIG1fc2NyaXB0RXhlY3V0 aW9uQ29udGV4dDsgfQo= 57599 2010-06-01 15:02:57 -0700 2010-06-02 01:45:56 -0700 Move the adoptRef to Notification::create() 39998.patch text/plain 3553 yael SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MDUwOSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjIgQEAKKzIwMTAtMDYtMDEgIFlhZWwgQWhhcm9uICA8eWFlbC5haGFyb25Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE5vdGlmaWNhdGlvbiBvYmplY3QgcmVmIGNvdW50aW5nIGlzIG5vdCBjb3JyZWN0LgorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9Mzk5OTgKKworICAgICAg ICBSZXR1cm4gUGFzc1JlZlB0cjxOb3RpZmljYXRpb24+IGluc3RlYWQgb2YgYSByYXcgcG9pbnRl ciBmcm9tIHRoZSBjcmVhdGUgbWV0aG9kcworICAgICAgICBmb3IgV2ViIE5vdGlmaWNhdGlvbnMu IAorCisgICAgICAgIE5vIG5ldyB0ZXN0cyBhcmUgbmVlZGVkLCBhcyBleGlzdGluZyB0ZXN0cyBz aG93IHRoZSBwcm9ibGVtIHdoZW4gc2VuZGluZyB0aGUgCisgICAgICAgICJkaXNwbGF5IiBldmVu dCBhc3luY2hyb25vdXNseS4KKworICAgICAgICAqIG5vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9u Lmg6CisgICAgICAgIChXZWJDb3JlOjpOb3RpZmljYXRpb246OmNyZWF0ZSk6CisgICAgICAgICog bm90aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25DZW50ZXIuaDoKKyAgICAgICAgKFdlYkNvcmU6Ok5v dGlmaWNhdGlvbkNlbnRlcjo6Y3JlYXRlSFRNTE5vdGlmaWNhdGlvbik6CisgICAgICAgIChXZWJD b3JlOjpOb3RpZmljYXRpb25DZW50ZXI6OmNyZWF0ZU5vdGlmaWNhdGlvbik6CisKIDIwMTAtMDYt MDEgIERhdmlkIEh5YXR0ICA8aHlhdHRAYXBwbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5 IEFuZGVycyBDYXJsc3Nvbi4KSW5kZXg6IFdlYkNvcmUvbm90aWZpY2F0aW9ucy9Ob3RpZmljYXRp b24uaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL25vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9uLmgJ KHJldmlzaW9uIDYwNDc4KQorKysgV2ViQ29yZS9ub3RpZmljYXRpb25zL05vdGlmaWNhdGlvbi5o CSh3b3JraW5nIGNvcHkpCkBAIC01NSw4ICs1NSw4IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAog ICAgIGNsYXNzIE5vdGlmaWNhdGlvbiA6IHB1YmxpYyBSZWZDb3VudGVkPE5vdGlmaWNhdGlvbj4s IHB1YmxpYyBBY3RpdmVET01PYmplY3QsIHB1YmxpYyBFdmVudFRhcmdldCB7IAogICAgIHB1Ymxp YzoKLSAgICAgICAgc3RhdGljIE5vdGlmaWNhdGlvbiogY3JlYXRlKGNvbnN0IEtVUkwmIHVybCwg U2NyaXB0RXhlY3V0aW9uQ29udGV4dCogY29udGV4dCwgRXhjZXB0aW9uQ29kZSYgZWMsIE5vdGlm aWNhdGlvblByZXNlbnRlciogcHJvdmlkZXIpIHsgcmV0dXJuIG5ldyBOb3RpZmljYXRpb24odXJs LCBjb250ZXh0LCBlYywgcHJvdmlkZXIpOyB9Ci0gICAgICAgIHN0YXRpYyBOb3RpZmljYXRpb24q IGNyZWF0ZShjb25zdCBOb3RpZmljYXRpb25Db250ZW50cyYgY29udGVudHMsIFNjcmlwdEV4ZWN1 dGlvbkNvbnRleHQqIGNvbnRleHQsIEV4Y2VwdGlvbkNvZGUmIGVjLCBOb3RpZmljYXRpb25QcmVz ZW50ZXIqIHByb3ZpZGVyKSB7IHJldHVybiBuZXcgTm90aWZpY2F0aW9uKGNvbnRlbnRzLCBjb250 ZXh0LCBlYywgcHJvdmlkZXIpOyB9CisgICAgICAgIHN0YXRpYyBQYXNzUmVmUHRyPE5vdGlmaWNh dGlvbj4gY3JlYXRlKGNvbnN0IEtVUkwmIHVybCwgU2NyaXB0RXhlY3V0aW9uQ29udGV4dCogY29u dGV4dCwgRXhjZXB0aW9uQ29kZSYgZWMsIE5vdGlmaWNhdGlvblByZXNlbnRlciogcHJvdmlkZXIp IHsgcmV0dXJuIGFkb3B0UmVmKG5ldyBOb3RpZmljYXRpb24odXJsLCBjb250ZXh0LCBlYywgcHJv dmlkZXIpKTsgfQorICAgICAgICBzdGF0aWMgUGFzc1JlZlB0cjxOb3RpZmljYXRpb24+IGNyZWF0 ZShjb25zdCBOb3RpZmljYXRpb25Db250ZW50cyYgY29udGVudHMsIFNjcmlwdEV4ZWN1dGlvbkNv bnRleHQqIGNvbnRleHQsIEV4Y2VwdGlvbkNvZGUmIGVjLCBOb3RpZmljYXRpb25QcmVzZW50ZXIq IHByb3ZpZGVyKSB7IHJldHVybiBhZG9wdFJlZihuZXcgTm90aWZpY2F0aW9uKGNvbnRlbnRzLCBj b250ZXh0LCBlYywgcHJvdmlkZXIpKTsgfQogICAgICAgICAKICAgICAgICAgdmlydHVhbCB+Tm90 aWZpY2F0aW9uKCk7CiAKSW5kZXg6IFdlYkNvcmUvbm90aWZpY2F0aW9ucy9Ob3RpZmljYXRpb25D ZW50ZXIuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL25vdGlmaWNhdGlvbnMvTm90aWZpY2F0aW9u Q2VudGVyLmgJKHJldmlzaW9uIDYwNDc4KQorKysgV2ViQ29yZS9ub3RpZmljYXRpb25zL05vdGlm aWNhdGlvbkNlbnRlci5oCSh3b3JraW5nIGNvcHkpCkBAIC00OSw3ICs0OSw3IEBAIG5hbWVzcGFj ZSBXZWJDb3JlIHsKICAgICBwdWJsaWM6CiAgICAgICAgIHN0YXRpYyBQYXNzUmVmUHRyPE5vdGlm aWNhdGlvbkNlbnRlcj4gY3JlYXRlKFNjcmlwdEV4ZWN1dGlvbkNvbnRleHQqIGNvbnRleHQsIE5v dGlmaWNhdGlvblByZXNlbnRlciogcHJlc2VudGVyKSB7IHJldHVybiBhZG9wdFJlZihuZXcgTm90 aWZpY2F0aW9uQ2VudGVyKGNvbnRleHQsIHByZXNlbnRlcikpOyB9CiAKLSAgICAgICAgTm90aWZp Y2F0aW9uKiBjcmVhdGVIVE1MTm90aWZpY2F0aW9uKGNvbnN0IFN0cmluZyYgVVJJLCBFeGNlcHRp b25Db2RlJiBlYykKKyAgICAgICAgUGFzc1JlZlB0cjxOb3RpZmljYXRpb24+IGNyZWF0ZUhUTUxO b3RpZmljYXRpb24oY29uc3QgU3RyaW5nJiBVUkksIEV4Y2VwdGlvbkNvZGUmIGVjKQogICAgICAg ICB7CiAgICAgICAgICAgICBpZiAoIXByZXNlbnRlcigpKSB7CiAgICAgICAgICAgICAgICAgZWMg PSBJTlZBTElEX1NUQVRFX0VSUjsKQEAgLTYyLDcgKzYyLDcgQEAgbmFtZXNwYWNlIFdlYkNvcmUg ewogICAgICAgICAgICAgcmV0dXJuIE5vdGlmaWNhdGlvbjo6Y3JlYXRlKG1fc2NyaXB0RXhlY3V0 aW9uQ29udGV4dC0+Y29tcGxldGVVUkwoVVJJKSwgY29udGV4dCgpLCBlYywgcHJlc2VudGVyKCkp OwogICAgICAgICB9CiAKLSAgICAgICAgTm90aWZpY2F0aW9uKiBjcmVhdGVOb3RpZmljYXRpb24o Y29uc3QgU3RyaW5nJiBpY29uVVJJLCBjb25zdCBTdHJpbmcmIHRpdGxlLCBjb25zdCBTdHJpbmcm IGJvZHksIEV4Y2VwdGlvbkNvZGUmIGVjKQorICAgICAgICBQYXNzUmVmUHRyPE5vdGlmaWNhdGlv bj4gY3JlYXRlTm90aWZpY2F0aW9uKGNvbnN0IFN0cmluZyYgaWNvblVSSSwgY29uc3QgU3RyaW5n JiB0aXRsZSwgY29uc3QgU3RyaW5nJiBib2R5LCBFeGNlcHRpb25Db2RlJiBlYykKICAgICAgICAg ewogICAgICAgICAgICAgaWYgKCFwcmVzZW50ZXIoKSkgewogICAgICAgICAgICAgICAgIGVjID0g SU5WQUxJRF9TVEFURV9FUlI7Cg==