Attempting to fix problems in 11571014.

Attempting to fix problems in 11571014.

[not at google - send to devlin, 2013-03-05 20:31:51]

This is a diff against cduvall's https://codereview.chromium.org/11571014/ to
hopefully fix failures there (and add some debugging - it was crashing in
canary) - but it became pretty substantial so I think it should be reviewed.

I can also upload it to that code review if you'd prefer.

[Matt Perry, 2013-03-05 21:51:38]

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
File chrome/renderer/extensions/module_system.cc (right):

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
chrome/renderer/extensions/module_system.cc:277: char stack_debug[64];
you might need to use base::Alias or whatever to make this available in crash
dumps.

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
File chrome/renderer/extensions/module_system.h (right):

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
chrome/renderer/extensions/module_system.h:160: typedef v8::Handle<v8::Value>
(ModuleSystem::*RequireFn)(
don't abbrev. RequireFunction

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/resources/...
File chrome/renderer/resources/extensions/send_request.js (right):

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/send_request.js:18: var request =
requests[requestId];
Why pull all this out of the try? Might as well be safe and keep it all in there
in case any of this throws.

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/send_request.js:25: request.callback ||
request.customCallback || chrome).chrome;
does GetGlobal() return the global object, or chrome? lastError.run() seems to
expect it to return chrome.

[not at google - send to devlin, 2013-03-05 22:05:37]

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
File chrome/renderer/extensions/module_system.cc (right):

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
chrome/renderer/extensions/module_system.cc:277: char stack_debug[64];
On 2013/03/05 21:51:38, Matt Perry wrote:
> you might need to use base::Alias or whatever to make this available in crash
> dumps.

Done.

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
File chrome/renderer/extensions/module_system.h (right):

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/extensions...
chrome/renderer/extensions/module_system.h:160: typedef v8::Handle<v8::Value>
(ModuleSystem::*RequireFn)(
On 2013/03/05 21:51:38, Matt Perry wrote:
> don't abbrev. RequireFunction

Done.

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/resources/...
File chrome/renderer/resources/extensions/send_request.js (right):

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/send_request.js:18: var request =
requests[requestId];
On 2013/03/05 21:51:38, Matt Perry wrote:
> Why pull all this out of the try? Might as well be safe and keep it all in
there
> in case any of this throws.

I pulled this out because the finally{} block references callerChrome, which
only makes sense if it's assigned outside the try scope. I'll do it in 2 stages.

Done.

https://codereview.chromium.org/12378077/diff/8001/chrome/renderer/resources/...
chrome/renderer/resources/extensions/send_request.js:25: request.callback ||
request.customCallback || chrome).chrome;
On 2013/03/05 21:51:38, Matt Perry wrote:
> does GetGlobal() return the global object, or chrome? lastError.run() seems to
> expect it to return chrome.

Yeah, chrome. There is a .chrome at the end of the natives.GetGlobal there, but
I'll move it to be more explicit.

[Matt Perry, 2013-03-05 22:20:19]

lgtm