Die on an OOM situation in many more cases....

Die on an OOM situation in many more cases.

Continuation of http://codereview.chromium.org/875004 after a revert.

BUG=http://crbug.com/12673
TEST=run out of memory and die


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=42127

[Avi (use Gerrit), 2010-03-17 21:41:43]

I don't think it's the same situation as the article you mentioned (superclass
implementations, etc) because NSObject has no superclass. I don't know why doing
a normal objc_msgSend isn't working in the plugin case, but this feels safer,
not calling back into the runtime while intercepting a core message.

[Avi (use Gerrit), 2010-03-17 21:44:05]

http://codereview.chromium.org/1039007/diff/4001/5004
File base/process_util_mac.mm (right):

http://codereview.chromium.org/1039007/diff/4001/5004#newcode584
base/process_util_mac.mm:584: 
removed blank line here.

[Mark Mentovai, 2010-03-17 22:31:43]

OK.

Before I LG this, for curiosity's sake, did you try the alternate strategy that
the article suggested?

[Avi (use Gerrit), 2010-03-17 22:34:09]

Which alternate strategy do you refer to?

There's no simple way to add a class method in the Obj2 runtime.

[Mark Mentovai, 2010-03-17 22:37:20]

avi@chromium.org wrote:
> Which alternate strategy do you refer to?
>
> There's no simple way to add a class method in the Obj2 runtime.
>
> http://codereview.chromium.org/1039007

From
http://www.mikeash.com/pyblog/friday-qa-2010-01-29-method-replacement-for-fun...:

        Method origMethod = class_getInstanceMethod(c, origSEL);
        Method overrideMethod = class_getInstanceMethod(c, overrideSEL);

        if(class_addMethod(c, origSEL,
method_getImplementation(overrideMethod),
method_getTypeEncoding(overrideMethod)))
        {
            class_replaceMethod(c, overrideSEL,
method_getImplementation(origMethod),
method_getTypeEncoding(origMethod));
        }
        else
        {
            method_exchangeImplementations(origMethod, overrideMethod);
        }

Mark

[Avi (use Gerrit), 2010-03-17 22:39:17]

These aren't instance methods; the first two variables would be nil.

[Mark Mentovai, 2010-03-17 22:41:34]

avi@chromium.org wrote:
> These aren't instance methods; the first two variables would be nil.

Sure, you'd substitute class_getClassMethod.

I guess that means you didn't try it?  I'd be curious to know if it
had any impact.

Mark

[Avi (use Gerrit), 2010-03-17 22:47:16]

Right, but class_addMethod and class_replaceMethod are for instance methods.
Couldn't find class versions, and in the past when I tried to use instance
method calls on metaclasses to get them to change the class methods, it didn't
work.

[Mark Mentovai, 2010-03-18 16:56:44]

avi@chromium.org wrote:
> Right, but class_addMethod and class_replaceMethod are for instance methods.
> Couldn't find class versions, and in the past when I tried to use instance
> method calls on metaclasses to get them to change the class methods, it
> didn't work.

I'm pretty sure I've used them for class methods.

In any event, could you just give it a quick try?  The unit test crash
that we experienced was weird enough that I think it's a good idea for
us to try to figure out what is and isn't possible.

Swizzling is both easier and harder since 10.5...

Mark

[Avi (use Gerrit), 2010-03-18 19:37:54]

For the record (we talked in person) addMethod fails and then we'd fall through
to the exchangeImplementations (which is known to fail and break all the plugin
ui tests, though it's still unclear why).

[Mark Mentovai, 2010-03-18 19:43:10]

LGTM, this should work in any event.  The question is just one more bit of
information that'd be nice to know.

http://codereview.chromium.org/1039007/diff/4001/5004
File base/process_util_mac.mm (right):

http://codereview.chromium.org/1039007/diff/4001/5004#newcode577
base/process_util_mac.mm:577: @selector(allocWithZone:));
I'm still curious about what sel_getName(method_getName(orig_method)) would be
here - that might be the key to why this didn't work with
method_exchangeImplementations the first time you tried it.

[Avi (use Gerrit), 2010-03-18 19:56:31]

It's exactly what we'd expect: |allocWithZone:|