Remove unneeded extension_messages_browsertest.cc + cleanup.

Remove unneeded extension_messages_browsertest.cc + cleanup.


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

[Aaron Boodman, 2012-04-10 07:19:45]

https://chromiumcodereview.appspot.com/10024055/diff/1/chrome/renderer/extens...
File chrome/renderer/extensions/extension_dispatcher.cc (left):

https://chromiumcodereview.appspot.com/10024055/diff/1/chrome/renderer/extens...
chrome/renderer/extensions/extension_dispatcher.cc:652: // TODO(kalman): this is
probably the most unfortunate thing I've ever had to
You've had an easy life my friend.

[not at google - send to devlin, 2012-04-10 20:42:02]

lgtm

http://codereview.chromium.org/10024055/diff/1/chrome/renderer/extensions/ext...
File chrome/renderer/extensions/extension_dispatcher.cc (left):

http://codereview.chromium.org/10024055/diff/1/chrome/renderer/extensions/ext...
chrome/renderer/extensions/extension_dispatcher.cc:652: // TODO(kalman): this is
probably the most unfortunate thing I've ever had to
On 2012/04/10 07:19:46, Aaron Boodman wrote:
> You've had an easy life my friend.

I just have a high depreciation of bad things.

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/api_definitions_natives.cc (left):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/api_definitions_natives.cc:31: // We need to remove
that first.
yay!

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/chrome_v8_context.h (right):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/chrome_v8_context.h:33: const Extension* extension,
shouldn't this and extension_ be a scoped_refptr?

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/chrome_v8_context.h:98: // APIs are available,
returns an empty set, not NULL.
Return a const pointer?

Or even a reference, if it's kosher to return a reference into a scoped_ptr.

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/chrome_v8_context_set.cc (right):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/chrome_v8_context_set.cc:121: if (!(*it)->extension()
|| (extension_id != (*it)->extension()->id()))
nit: saving a local reference to the extension might be a bit cleaner.

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_dispatcher.cc (right):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_dispatcher.cc:642: ++i) {
I think it was right before

[not at google - send to devlin, 2012-04-10 20:42:12]

and btw, totally awesome.

[Aaron Boodman, 2012-04-10 21:02:04]

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/chrome_v8_context.h (right):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/chrome_v8_context.h:33: const Extension* extension,
On 2012/04/10 20:42:02, kalman wrote:
> shouldn't this and extension_ be a scoped_refptr?

scoped_refptr makes me nervous. I prefer to use it only in cases where the
lifetime should actually be controlled.

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/chrome_v8_context.h:98: // APIs are available,
returns an empty set, not NULL.
On 2012/04/10 20:42:02, kalman wrote:
> Return a const pointer?
> 
> Or even a reference, if it's kosher to return a reference into a scoped_ptr.

Good idea. Dizzone.

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/chrome_v8_context_set.cc (right):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/chrome_v8_context_set.cc:121: if (!(*it)->extension()
|| (extension_id != (*it)->extension()->id()))
On 2012/04/10 20:42:02, kalman wrote:
> nit: saving a local reference to the extension might be a bit cleaner.

Done.

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
File chrome/renderer/extensions/extension_dispatcher.cc (right):

http://codereview.chromium.org/10024055/diff/2001/chrome/renderer/extensions/...
chrome/renderer/extensions/extension_dispatcher.cc:642: ++i) {
On 2012/04/10 20:42:02, kalman wrote:
> I think it was right before

It must be weird to be so wrong all the time!

[commit-bot: I haz the power, 2012-04-10 21:03:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/10024055/5004

[commit-bot: I haz the power, 2012-04-10 21:54:41]

Try job failure for 10024055-5004 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-04-11 06:32:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/aa@chromium.org/10024055/10001

[commit-bot: I haz the power, 2012-04-11 07:58:32]

Change committed as 131730