54257 2011-02-10 16:18:48 -0800 Make DefaultGCActivityCallback for PLATFORM(CF) Easier to Subclass 2011-02-10 17:18:32 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) PC OS X 10.5 RESOLVED FIXED P2 Normal --- 1 joepeck webkit-unassigned joepeck psolanki oldest_to_newest 349487 0 joepeck 2011-02-10 16:18:48 -0800 Allow the CFRunLoop to be specified in a constructor. 349497 1 82071 joepeck 2011-02-10 16:32:48 -0800 Created attachment 82071 [PATCH] Proposed Solution I'm open to suggestions for a better name for "commonConstructor". Also I'm fine with making that private. 349506 2 82071 ggaren 2011-02-10 16:46:18 -0800 Comment on attachment 82071 [PATCH] Proposed Solution View in context: https://bugs.webkit.org/attachment.cgi?id=82071&action=review r=me > Source/JavaScriptCore/ChangeLog:8 > + A subclass may want to specify the CFRunLoop the Garbage Collection. It Typo, I think. > Source/JavaScriptCore/runtime/GCActivityCallbackCF.cpp:86 > + memset(&d->context, '\0', sizeof(CFRunLoopTimerContext)); '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'. 349517 3 joepeck 2011-02-10 17:10:12 -0800 Landed in r78291: http://trac.webkit.org/changeset/78291 349518 4 joepeck 2011-02-10 17:10:44 -0800 > > Source/JavaScriptCore/ChangeLog:8 > > + A subclass may want to specify the CFRunLoop the Garbage Collection. It > > Typo, I think. Yep, I cleaned up my ChangeLog mistakes. > > Source/JavaScriptCore/runtime/GCActivityCallbackCF.cpp:86 > > + memset(&d->context, '\0', sizeof(CFRunLoopTimerContext)); > > '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'. Done. Thanks! 349520 5 psolanki 2011-02-10 17:14:17 -0800 (In reply to comment #4) > > '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'. > > Done. Thanks! I think Geoff meant to say pass 0, not '0'. Passing '0' to memset will initialize the memory to ASCII character 0. 349526 6 joepeck 2011-02-10 17:18:32 -0800 (In reply to comment #5) > (In reply to comment #4) > > > '\0' is a strange way to say '0'. I know you didn't write this code initially, but you might as well change it to '0'. > > > > Done. Thanks! > > I think Geoff meant to say pass 0, not '0'. Passing '0' to memset will initialize the memory to ASCII character 0. Yes, entirely my fault. I took that literally. I landed a follow-up fix. Unreviewed since initially this was a review comment. http://trac.webkit.org/changeset/78292 Thanks! 82071 2011-02-10 16:32:48 -0800 2011-02-10 16:46:18 -0800 [PATCH] Proposed Solution jsc.patch text/plain 4329 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDVhZTA1MGMuLjdlMmZlOWEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2IEBACisyMDExLTAyLTEwICBKb3NlcGggUGVjb3Jh cm8gIDxqb2VwZWNrQHdlYmtpdC5vcmc+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChP T1BTISkuCisKKyAgICAgICAgTWFrZSBEZWZhdWx0R0NBY3Rpdml0eUNhbGxiYWNrIGZvciBQTEFU Rk9STShDRikgRWFzaWVyIHRvIFN1YmNsYXNzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD01NDI1NworCisgICAgICAgIEEgc3ViY2xhc3MgbWF5IHdhbnQg dG8gc3BlY2lmeSB0aGUgQ0ZSdW5Mb29wIHRoZSBHYXJiYWdlIENvbGxlY3Rpb24uIEl0CisgICAg ICAgIGlzIGRpZmZpY3VsdCB0byBtYW5pcHVsYXRlIHRoaXMgaW4gYSBzdWJjbGFzcyBiZWNhdXNl IHRoZSBjdXJyZW50CisgICAgICAgIERlZmF1bHRHQ0FjdGl2aXR5Q2FsbGJhY2sgY2xhc3MgZG9l cyB0aGlzIGluIGl0cyBjb25zdHJ1Y3Rvci4gR2VuZXJhbGl6ZQorICAgICAgICB0aGlzIGEgYml0 IG1vcmUgc28gdGhhdCBhIHNwZWNpZmljIHJ1biBsb29wIGNhbiBiZSBwYXNzZWQgaW4gdG8gdGhl CisgICAgICAgIGNvbnN0cnVjdG9yLiBUaGlzIG1ha2VzIGl0IHNvIGFsbCBydW4gbG9vcCBtYW5h Z2VtZW50IGNhbiBzdGF5IGluCisgICAgICAgIERlZmF1bHRHQ0FjdGl2aXR5Q2FsbGJhY2sgYW5k IGEgc3ViY2xhc3MgY2FuIHNwZWNpZnkgYW55IHJ1bmxvb3AuCisKKyAgICAgICAgKiBydW50aW1l L0dDQWN0aXZpdHlDYWxsYmFjay5oOiBleHBvc2UgYSBQTEFURk9STShDRikgY29uc3RydWN0b3Ig dGhhdCBjYW4KKyAgICAgICAgc3BlY2lmeSB0aGUgcnVubG9vcCBHQyBjYW4gYmUgc2NoZWR1bGVk IG9uLgorICAgICAgICAqIHJ1bnRpbWUvR0NBY3Rpdml0eUNhbGxiYWNrQ0YuY3BwOgorICAgICAg ICAoSlNDOjpEZWZhdWx0R0NBY3Rpdml0eUNhbGxiYWNrOjpEZWZhdWx0R0NBY3Rpdml0eUNhbGxi YWNrKTogdHdvIGNvbnN0cnVjdG9ycywKKyAgICAgICAgb25lIHRoYXQgc3BlY2lmaWVzIHRoZSBy dW4gbG9vcCBhbmQgcGFzc2VzIHRoYXQgb24gdG8gY29tbW9uQ29uc3RydWN0b3IgYW5kCisgICAg ICAgIHRoZSBvbGQgY29uc3RydWN0b3IgZGVmYXVsdHMgdG8gdGhlIGN1cnJlbnQgcnVuIGxvb3Au CisgICAgICAgIChKU0M6OkRlZmF1bHRHQ0FjdGl2aXR5Q2FsbGJhY2s6OmNvbW1vbkNvbnN0cnVj dG9yKTogZG8gbm9ybWFsIGNvbnN0cnVjdGlvbiwKKyAgICAgICAgYWRkaW5nIHRoZSB0aW1lciB0 byB0aGUgZ2l2ZW4gcnVubG9vcC4KKwogMjAxMS0wMi0xMCAgUGF0cmljayBHYW5zdGVyZXIgIDxw YXJvZ2FAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbmRyZWFzIEtsaW5nLgpk aWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3J1bnRpbWUvR0NBY3Rpdml0eUNhbGxi YWNrLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9HQ0FjdGl2aXR5Q2FsbGJhY2su aAppbmRleCA4NjJiNGRmLi45YjZlZjA0IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9HQ0FjdGl2aXR5Q2FsbGJhY2suaAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvcnVudGltZS9HQ0FjdGl2aXR5Q2FsbGJhY2suaApAQCAtMzIsNiArMzIsMTAgQEAKICNpbmNs dWRlIDx3dGYvT3duUHRyLmg+CiAjaW5jbHVkZSA8d3RmL1Bhc3NPd25QdHIuaD4KIAorI2lmIFBM QVRGT1JNKENGKQorI2luY2x1ZGUgPENvcmVGb3VuZGF0aW9uL0NvcmVGb3VuZGF0aW9uLmg+Cisj ZW5kaWYKKwogbmFtZXNwYWNlIEpTQyB7CiAKIGNsYXNzIEhlYXA7CkBAIC01OCw2ICs2MiwxMiBA QCBwdWJsaWM6CiAgICAgdm9pZCBvcGVyYXRvcigpKCk7CiAgICAgdm9pZCBzeW5jaHJvbml6ZSgp OwogCisjaWYgUExBVEZPUk0oQ0YpCitwcm90ZWN0ZWQ6CisgICAgRGVmYXVsdEdDQWN0aXZpdHlD YWxsYmFjayhIZWFwKiwgQ0ZSdW5Mb29wUmVmKTsKKyAgICB2b2lkIGNvbW1vbkNvbnN0cnVjdG9y KEhlYXAqLCBDRlJ1bkxvb3BSZWYpOworI2VuZGlmCisKIHByaXZhdGU6CiAgICAgT3duUHRyPERl ZmF1bHRHQ0FjdGl2aXR5Q2FsbGJhY2tQbGF0Zm9ybURhdGEqPiBkOwogfTsKZGlmZiAtLWdpdCBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0dDQWN0aXZpdHlDYWxsYmFja0NGLmNwcCBi L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL0dDQWN0aXZpdHlDYWxsYmFja0NGLmNwcApp bmRleCA0MTRhMGFkLi5jODdlM2VlIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv cnVudGltZS9HQ0FjdGl2aXR5Q2FsbGJhY2tDRi5jcHAKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL3J1bnRpbWUvR0NBY3Rpdml0eUNhbGxiYWNrQ0YuY3BwCkBAIC0zNSw3ICszNSw2IEBACiAj aW5jbHVkZSAiSlNMb2NrLmgiCiAjaW5jbHVkZSA8d3RmL1JldGFpblB0ci5oPgogI2luY2x1ZGUg PHd0Zi9XVEZUaHJlYWREYXRhLmg+Ci0jaW5jbHVkZSA8Q29yZUZvdW5kYXRpb24vQ29yZUZvdW5k YXRpb24uaD4KIAogI2lmICFQTEFURk9STShDRikKICNlcnJvciAiVGhpcyBmaWxlIHNob3VsZCBv bmx5IGJlIHVzZWQgb24gQ0YgcGxhdGZvcm1zLiIKQEAgLTYzLDEzICs2MiwxMiBAQCB2b2lkIERl ZmF1bHRHQ0FjdGl2aXR5Q2FsbGJhY2tQbGF0Zm9ybURhdGE6OnRyaWdnZXIoQ0ZSdW5Mb29wVGlt ZXJSZWYsIHZvaWQgKmluZgogCiBEZWZhdWx0R0NBY3Rpdml0eUNhbGxiYWNrOjpEZWZhdWx0R0NB Y3Rpdml0eUNhbGxiYWNrKEhlYXAqIGhlYXApCiB7Ci0gICAgZCA9IGFkb3B0UHRyKG5ldyBEZWZh dWx0R0NBY3Rpdml0eUNhbGxiYWNrUGxhdGZvcm1EYXRhKTsKKyAgICBjb21tb25Db25zdHJ1Y3Rv cihoZWFwLCBDRlJ1bkxvb3BHZXRDdXJyZW50KCkpOworfQogCi0gICAgbWVtc2V0KCZkLT5jb250 ZXh0LCAnXDAnLCBzaXplb2YoQ0ZSdW5Mb29wVGltZXJDb250ZXh0KSk7Ci0gICAgZC0+Y29udGV4 dC5pbmZvID0gaGVhcDsKLSAgICBkLT5ydW5Mb29wID0gQ0ZSdW5Mb29wR2V0Q3VycmVudCgpOwot ICAgIGQtPnRpbWVyLmFkb3B0Q0YoQ0ZSdW5Mb29wVGltZXJDcmVhdGUoMCwgZGVjYWRlLCBkZWNh ZGUsIDAsIDAsIERlZmF1bHRHQ0FjdGl2aXR5Q2FsbGJhY2tQbGF0Zm9ybURhdGE6OnRyaWdnZXIs ICZkLT5jb250ZXh0KSk7Ci0gICAgQ0ZSdW5Mb29wQWRkVGltZXIoZC0+cnVuTG9vcC5nZXQoKSwg ZC0+dGltZXIuZ2V0KCksIGtDRlJ1bkxvb3BDb21tb25Nb2Rlcyk7CitEZWZhdWx0R0NBY3Rpdml0 eUNhbGxiYWNrOjpEZWZhdWx0R0NBY3Rpdml0eUNhbGxiYWNrKEhlYXAqIGhlYXAsIENGUnVuTG9v cFJlZiBydW5Mb29wKQoreworICAgIGNvbW1vbkNvbnN0cnVjdG9yKGhlYXAsIHJ1bkxvb3ApOwog fQogCiBEZWZhdWx0R0NBY3Rpdml0eUNhbGxiYWNrOjp+RGVmYXVsdEdDQWN0aXZpdHlDYWxsYmFj aygpCkBAIC04MSw2ICs3OSwxNyBAQCBEZWZhdWx0R0NBY3Rpdml0eUNhbGxiYWNrOjp+RGVmYXVs dEdDQWN0aXZpdHlDYWxsYmFjaygpCiAgICAgZC0+dGltZXIgPSAwOwogfQogCit2b2lkIERlZmF1 bHRHQ0FjdGl2aXR5Q2FsbGJhY2s6OmNvbW1vbkNvbnN0cnVjdG9yKEhlYXAqIGhlYXAsIENGUnVu TG9vcFJlZiBydW5Mb29wKQoreworICAgIGQgPSBhZG9wdFB0cihuZXcgRGVmYXVsdEdDQWN0aXZp dHlDYWxsYmFja1BsYXRmb3JtRGF0YSk7CisKKyAgICBtZW1zZXQoJmQtPmNvbnRleHQsICdcMCcs IHNpemVvZihDRlJ1bkxvb3BUaW1lckNvbnRleHQpKTsKKyAgICBkLT5jb250ZXh0LmluZm8gPSBo ZWFwOworICAgIGQtPnJ1bkxvb3AgPSBydW5Mb29wOworICAgIGQtPnRpbWVyLmFkb3B0Q0YoQ0ZS dW5Mb29wVGltZXJDcmVhdGUoMCwgZGVjYWRlLCBkZWNhZGUsIDAsIDAsIERlZmF1bHRHQ0FjdGl2 aXR5Q2FsbGJhY2tQbGF0Zm9ybURhdGE6OnRyaWdnZXIsICZkLT5jb250ZXh0KSk7CisgICAgQ0ZS dW5Mb29wQWRkVGltZXIoZC0+cnVuTG9vcC5nZXQoKSwgZC0+dGltZXIuZ2V0KCksIGtDRlJ1bkxv b3BDb21tb25Nb2Rlcyk7Cit9CisKIHZvaWQgRGVmYXVsdEdDQWN0aXZpdHlDYWxsYmFjazo6b3Bl cmF0b3IoKSgpCiB7CiAgICAgQ0ZSdW5Mb29wVGltZXJTZXROZXh0RmlyZURhdGUoZC0+dGltZXIu Z2V0KCksIENGQWJzb2x1dGVUaW1lR2V0Q3VycmVudCgpICsgdHJpZ2dlckludGVydmFsKTsK