Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
7084017:
[Mac] Use object_destructInstance() if available. (Closed)
|
Created:
9 years, 7 months ago by Scott Hess - ex-Googler Modified:
9 years, 6 months ago CC:
chromium-reviews, pam+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
Description[Mac] Use object_destructInstance() if available.
10.6 added object_destructInstance() as a wrapper around
object_cxxDestruct() and also releasing associated objects. As a
bonus, it's available via dlsym() rather than nlist().
BUG=82937
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88378
Patch Set 1 #
Total comments: 15
Patch Set 2 : Mark's changes. #
Total comments: 10
Patch Set 3 : Per-runtime initialization, unit test, misc. #
Total comments: 13
Patch Set 4 : Push runtime version check down into ZombieInit(). #
Total comments: 5
Patch Set 5 : Avi grammar nits. #
Messages
Total messages: 13 (0 generated)
http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:86: return (DestructFn*)fn; I need to clean up the casting in this function, and I'll need to wait until I'm at the office next week to test against 10.5. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:135: // won't hurt anything. Sorry, I think this comment snuck over when I broke my git branch into two CLs.
This will need a rebase, but it’s good too. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:41: typedef void DestructFn(id obj); Technically the function pointer types are incompatible, because objc_destructInstance returns void* and object_cxxDestruct returns void. Since we don’t care about the return value, it should be safe to type both as void as you’ve done here to keep the code simpler, and that ought to be fine on the architectures we care about. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:77: #if ARCH_CPU_64_BITS Put this below the dlsym (but keep it before the nlist). Since I doubt we’ll ever support 10.5/64, maybe also get rid of the NOTIMPLEMENTED and resolve bug 44021. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:135: // won't hurt anything. shess wrote: > Sorry, I think this comment snuck over when I broke my git branch into two CLs. That’s OK, it’s obvious that this patch isn’t based on the other, but when you do rebase this one, this will show up as a change to the comment you wrote in the other, because you can’t mention object_cxxDestruct here with this change as you did there.
Here are two things I thought of during the day. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:84: void* fn = dlsym(RTLD_DEFAULT, "objc_destructInstance"); RTLD_DEFAULT kind of sucks for a few reasons. It’ll search all loaded libraries’ symbol tables to find a match (and we wouldn’t expect a match on 10.5, so it’d search everything each time) and it defeats the “two-level” lookup that dyld normally does when it binds symbols for us, subjecting us to possibly picking up the wrong symbol here. I think we should use a real handle from dlopen. The dlopen can either use the path to libobjc.dylib determined by a dladdr call to look up something known to be in libobjc, or can just hard-code the known path to libobjc.dylib. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:88: struct nlist nl[3]; If we can’t find objc_destructInstance with dlsym and we’re NOT on 10.5, I think we should LOG(WARNING) something. Otherwise, since we’re dealing with private APIs, we might not know if things stop working as we intend in some future system version.
http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:41: typedef void DestructFn(id obj); On 2011/05/28 13:20:46, Mark Mentovai wrote: > Technically the function pointer types are incompatible, because > objc_destructInstance returns void* and object_cxxDestruct returns void. Since > we don’t care about the return value, it should be safe to type both as void as > you’ve done here to keep the code simpler, and that ought to be fine on the > architectures we care about. Augmented the comment to that end. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:77: #if ARCH_CPU_64_BITS On 2011/05/28 13:20:46, Mark Mentovai wrote: > Put this below the dlsym (but keep it before the nlist). > > Since I doubt we’ll ever support 10.5/64, maybe also get rid of the > NOTIMPLEMENTED and resolve bug 44021. You sure? I think it's the right long-term decision, but the main reason I put this in here is so that someone revisits this file when we finally make the 64-bit transition. I didn't originally fix the problem as described because without anyone actually exercising the code, it seemed likely that it would bitrot. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:84: void* fn = dlsym(RTLD_DEFAULT, "objc_destructInstance"); On 2011/05/29 22:02:31, Mark Mentovai wrote: > RTLD_DEFAULT kind of sucks for a few reasons. It’ll search all loaded libraries’ > symbol tables to find a match (and we wouldn’t expect a match on 10.5, so it’d > search everything each time) and it defeats the “two-level” lookup that dyld > normally does when it binds symbols for us, subjecting us to possibly picking up > the wrong symbol here. > > I think we should use a real handle from dlopen. The dlopen can either use the > path to libobjc.dylib determined by a dladdr call to look up something known to > be in libobjc, or can just hard-code the known path to libobjc.dylib. Done. I do not know what I'm doing in this area, though, so review thoroughly. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:86: return (DestructFn*)fn; On 2011/05/28 05:44:35, shess wrote: > I need to clean up the casting in this function, and I'll need to wait until I'm > at the office next week to test against 10.5. OK, casting is so annoying that I'm not sure I'm doing things right. Is reinterpret_cast<>() the way to go, or am I missing something? AFAICT, 10.5 still works. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:88: struct nlist nl[3]; On 2011/05/29 22:02:31, Mark Mentovai wrote: > If we can’t find objc_destructInstance with dlsym and we’re NOT on 10.5, I think > we should LOG(WARNING) something. Otherwise, since we’re dealing with private > APIs, we might not know if things stop working as we intend in some future > system version. Earlier when I put in the whitelist, I left it way up in BrowserCrApplication because it felt wrong to have such dependencies way down here. But your suggestion isn't so much a change to operation as a diagnostic output... Given the tightly-bound nature versus the runtime, it's probably reasonable to have it failsafe. So the nlist path would ONLY be used for 10.5, the dlsym path for 10.6 with no fallback to nlist, and whatever is appropriate for future versions. WDYT? Maybe with a histogram to track failures?
This would be LG but I’m not giving it yet because it seems like you want to redo the “nlist for 10.5, dlsym for higher” thing. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:77: #if ARCH_CPU_64_BITS shess wrote: > On 2011/05/28 13:20:46, Mark Mentovai wrote: > > Put this below the dlsym (but keep it before the nlist). > > > > Since I doubt we’ll ever support 10.5/64, maybe also get rid of the > > NOTIMPLEMENTED and resolve bug 44021. > > You sure? Yeah. objc_destructInstance and the dlsym to get it will work on 10.6 x86_64. I seriously doubt we’ll need to support 10.5 x86_64. If there’s another problem with the code on x86_64, we’d have to find it anyway. I don’t see any reason to declare up front that “this code might not work in 64-bit mode because it’s untested” when the reality is that all of our code is untested in 64-bit mode and thus might not work. Why single this file out as special? The way it was before, it was a good idea to have the ARCH_CPU_64_BITS check, because you knew for a fact that nlist wouldn’t work in 64-bit mode. But now, I think it’ll be more of a distraction to the person doing the 64-bit porting (probably me) than anything else. We don’t know for a fact that it won’t work, and in fact, it most likely will work. However, what I wouldn’t mind seeing would be wrapping all of the nlist code inside #if ARCH_CPU_32_BITS. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:88: struct nlist nl[3]; shess wrote: > On 2011/05/29 22:02:31, Mark Mentovai wrote: > > If we can’t find objc_destructInstance with dlsym and we’re NOT on 10.5, I > think > > we should LOG(WARNING) something. Otherwise, since we’re dealing with private > > APIs, we might not know if things stop working as we intend in some future > > system version. > > Earlier when I put in the whitelist, I left it way up in BrowserCrApplication > because it felt wrong to have such dependencies way down here. But your > suggestion isn't so much a change to operation as a diagnostic output... > > Given the tightly-bound nature versus the runtime, it's probably reasonable to > have it failsafe. So the nlist path would ONLY be used for 10.5, the dlsym path > for 10.6 with no fallback to nlist, and whatever is appropriate for future > versions. WDYT? I think I can get behind that. I’d just use the 10.6 path as “10.6 and up,” taking care to warn if you can’t access the function you want, whether you used dlsym or nlist. > Maybe with a histogram to track failures? I doubt that’s necessary, but I won’t stop you if you want to. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:91: nl[0].n_un.n_name = (char*)"_object_cxxDestruct"; Not RTLD_FIRST. RTLD_LAZY | RTLD_LOCAL is fine. Note the space around the bitwise-or operator. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:94: // Use |class_addIvar| as a known reference point. This is normally dangerous. If you get a pointer from dlsym, it’s only guaranteed to be valid as long as the handle that you dlopened is. However, the use here is safe, because your dlopen is just incrementing the reference count of something that you already know is open, and isn’t going to be unloaded because it was loaded by dyld in response to a load command. A brief comment explaining that would be helpful. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:96: This cast is fine. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:111: // zombied. You could maybe use info.dli_fname here too, since you already have it? Or maybe you won’t already have it if this becomes the 10.5-only codepath and the other then 10.6-and-up codepath.
http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:116: reinterpret_cast<char*>(&class_addIvar) - nl[1].n_value + nl[0].n_value); Maybe intptr_t is a little more meaningful than char*.
Sorry for the long pause, I got side-tracked with unrelated M-13 stuff. And I also got off in the weeds a bit on how to structure the handling of runtime version. Fortunately, git means that you don't have to see all those weeds, just some of them. http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/1/chrome/browser/ui/cocoa/objc_zo... chrome/browser/ui/cocoa/objc_zombie.mm:77: #if ARCH_CPU_64_BITS On 2011/05/31 21:00:57, Mark Mentovai wrote: > However, what I wouldn’t mind seeing would be wrapping all of the nlist code > inside #if ARCH_CPU_32_BITS. Done. AFAICT, nlist() isn't present for 64-bit, and nlist_64 is only present for purposes of iterating the symbol table segment. So supporting this for 64-bit 10.5 would require adding code to iterate the symbols ourselves. Fun! http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:91: nl[0].n_un.n_name = (char*)"_object_cxxDestruct"; On 2011/05/31 21:00:57, Mark Mentovai wrote: > Not RTLD_FIRST. RTLD_LAZY | RTLD_LOCAL is fine. Note the space around the > bitwise-or operator. Done. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:94: // Use |class_addIvar| as a known reference point. On 2011/05/31 21:00:57, Mark Mentovai wrote: > This is normally dangerous. If you get a pointer from dlsym, it’s only > guaranteed to be valid as long as the handle that you dlopened is. However, the > use here is safe, because your dlopen is just incrementing the reference count > of something that you already know is open, and isn’t going to be unloaded > because it was loaded by dyld in response to a load command. > > A brief comment explaining that would be helpful. Done. You're making me think fondly about dlsym(RTLD_DEFAULT, "..."), though :-). http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:96: On 2011/05/31 21:00:57, Mark Mentovai wrote: > This cast is fine. OK. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:111: // zombied. On 2011/05/31 21:00:57, Mark Mentovai wrote: > You could maybe use info.dli_fname here too, since you already have it? Or maybe > you won’t already have it if this becomes the 10.5-only codepath and the other > then 10.6-and-up codepath. I did break the function across the two code paths, since they should never be shared in real life, but I can abstract that bit out. http://codereview.chromium.org/7084017/diff/6001/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:116: On 2011/05/31 21:05:05, Mark Mentovai wrote: > Maybe intptr_t is a little more meaningful than char*. Done. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_application_mac.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... chrome/browser/chrome_browser_application_mac.mm:246: } I am not really happy with this code. But it wasn't tons better written other ways. At least this way it reads top-to-bottom ...
http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_application_mac.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... chrome/browser/chrome_browser_application_mac.mm:209: UMA_HISTOGRAM_ENUMERATION("OSX.ZombieEnableFailure", failure, FAILED_MAX); I don’t remember if you need to edit some file to get these recognized properly. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... chrome/browser/chrome_browser_application_mac.mm:246: } shess wrote: > I am not really happy with this code. But it wasn't tons better written other > ways. At least this way it reads top-to-bottom ... Just a suggestion as to an alternative: you could have two variables, one for ObjcEvilDoers::RUNTIME_* and the other for FAILED_*, set them up properly, and then just have |if (!ZombieEnable(r)) RecordFailure(f);| once. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... File chrome/browser/ui/cocoa/objc_zombie.h (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie.h:19: RUNTIME_GUESS, This is only used in test code. Seems like you duplicate the OS-version detection for the same purpose in two locations, once in chrome_browser_application.mm (used in production) and once in objc_zombie.mm (used by RUNTIME_GUESS in tests). Not so hot. What if you left all of the detective work in objc_zombie.mm and had the zombie-setup code return a value that indicates success, failed-10.5, failed-10.6, failed-otherwise? http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie.mm:90: const char* objcRuntimePath = LookupObjcRuntimePath(); Use C++ naming rules in C++ code: objc_runtime_path. Also on line 112, and referenceAddr on line 131. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... File chrome/browser/ui/cocoa/objc_zombie_unittest.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie_unittest.mm:16: // Dynamically lookup |objc_setAssociatedObject()|, which isn't look up http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie_unittest.mm:46: { You don’t need the { } for an instance variable-free class. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie_unittest.mm:129: // When |soonInfected| becomes a zombie, the associated object Good test.
OK, I pulled the version-check stuff down into objc_zombies.mm, and ... well, it looks better all around. So though I still don't care for it on some abstract level, it seems like a reasonable way to go, rather than spraying weirdo code into the callers. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_application_mac.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/chrome_brows... chrome/browser/chrome_browser_application_mac.mm:209: UMA_HISTOGRAM_ENUMERATION("OSX.ZombieEnableFailure", failure, FAILED_MAX); On 2011/06/07 21:42:57, Mark Mentovai wrote: > I don’t remember if you need to edit some file to get these recognized properly. They're reported as hashes, so you need to tell another tool about the hashes. I'll follow up with jar on that. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... File chrome/browser/ui/cocoa/objc_zombie.h (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie.h:19: RUNTIME_GUESS, On 2011/06/07 21:42:57, Mark Mentovai wrote: > This is only used in test code. Seems like you duplicate the OS-version > detection for the same purpose in two locations, once in > chrome_browser_application.mm (used in production) and once in objc_zombie.mm > (used by RUNTIME_GUESS in tests). Not so hot. RUNTIME_10_x is for the specific runtime, while RUNTIME_GUESS tries for 10.6, and if it fails tries for 10.5. > What if you left all of the detective work in objc_zombie.mm and had the > zombie-setup code return a value that indicates success, failed-10.5, > failed-10.6, failed-otherwise? I'm having some brain mismatch in here. For some reason I feel like the zombie-setup code is more basic stuff that should provide capabilities but not make policy decisions. So it knows "This function is present via dlsym(), so we work in this case", but if that function is _not_ present, should that mean we have the wrong version, or that we should fallback to the older version? http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie.mm:90: const char* objcRuntimePath = LookupObjcRuntimePath(); On 2011/06/07 21:42:57, Mark Mentovai wrote: > Use C++ naming rules in C++ code: objc_runtime_path. > > Also on line 112, and referenceAddr on line 131. Done. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... File chrome/browser/ui/cocoa/objc_zombie_unittest.mm (right): http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie_unittest.mm:16: // Dynamically lookup |objc_setAssociatedObject()|, which isn't On 2011/06/07 21:42:57, Mark Mentovai wrote: > look up Done. http://codereview.chromium.org/7084017/diff/10001/chrome/browser/ui/cocoa/obj... chrome/browser/ui/cocoa/objc_zombie_unittest.mm:46: { On 2011/06/07 21:42:57, Mark Mentovai wrote: > You don’t need the { } for an instance variable-free class. Done.
http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:302: g_objectDestruct = LookupObjectDestruct_10_6(); Note that what's currently on trunk disables CrZombie for this case. In theory this works with 10.7, but I'm not sure we have anything that will provide a direct signal that it isn't working.
I think that this is good to roll with. LGTM. http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:302: g_objectDestruct = LookupObjectDestruct_10_6(); shess wrote: > Note that what's currently on trunk disables CrZombie for this case. In theory > this works with 10.7, but I'm not sure we have anything that will provide a > direct signal that it isn't working. Right. Once you check this in, we should do a follow-up to turn this back on for >10.6. I understand if you’re wary of doing this without testing on a Lion DP. If you don’t have a Lion installation and don’t have someone nearby who does, talk to me on Thursday and I’ll test it. (I’m out tomorrow.)
drive by grammar nits http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... File chrome/browser/ui/cocoa/objc_zombie.mm (right): http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:43: // |objc_destructInstance()| returns |void*|, pretending it has no "... returns |void*| but pretending..." http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie.mm:109: // Under 10.5 |object_cxxDestruct()| is used, unfortunately it is "used but unfortunately" http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... File chrome/browser/ui/cocoa/objc_zombie_unittest.mm (right): http://codereview.chromium.org/7084017/diff/7002/chrome/browser/ui/cocoa/objc... chrome/browser/ui/cocoa/objc_zombie_unittest.mm:58: - (id)initWithAssociatedObject:(id)anObject { nice! |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
