Adds browser-side component of Java Bridge for a TabContents

Adds browser-side component of Java Bridge for a TabContents

This patch adds JavaBridgeDispatcherHostManager, which co-ordinates injecting
Java objects into all RenderViews corresponding to a TabContents. The class is
owned by TabContents and manages one instance of JavaBridgeDispatcherHost for
each RenderViewHost.

Also adds TabContentsObserver::RenderViewDeleted().

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

[jam, 2011-10-21 16:18:20]

http://codereview.chromium.org/8366034/diff/3001/content/browser/renderer_hos...
File content/browser/renderer_host/java_bridge_dispatcher_host_manager.cc
(right):

http://codereview.chromium.org/8366034/diff/3001/content/browser/renderer_hos...
content/browser/renderer_host/java_bridge_dispatcher_host_manager.cc:54:
DCHECK(render_view_host);
this isn't necessary

http://codereview.chromium.org/8366034/diff/3001/content/browser/renderer_hos...
File content/browser/renderer_host/java_bridge_dispatcher_host_manager.h
(right):

http://codereview.chromium.org/8366034/diff/3001/content/browser/renderer_hos...
content/browser/renderer_host/java_bridge_dispatcher_host_manager.h:14: #include
<map>
nit: this goes first

http://codereview.chromium.org/8366034/diff/3001/content/browser/renderer_hos...
content/browser/renderer_host/java_bridge_dispatcher_host_manager.h:39: friend
class scoped_ptr<JavaBridgeDispatcherHostManager>;
i haven't seen this pattern before with scoped_ptr

http://codereview.chromium.org/8366034/diff/3001/content/browser/tab_contents...
File content/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/8366034/diff/3001/content/browser/tab_contents...
content/browser/tab_contents/tab_contents.cc:214:
java_bridge_dispatcher_host_manager_.reset(
we don't need to add this for all of chrome, do we? i.e. it seems we don't need
to link this in unless a port is using this java feature

[Steve Block, 2011-10-21 16:27:47]

http://codereview.chromium.org/8366034/diff/3001/content/browser/tab_contents...
File content/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/8366034/diff/3001/content/browser/tab_contents...
content/browser/tab_contents/tab_contents.cc:214:
java_bridge_dispatcher_host_manager_.reset(
No, this isn't needed unless we want to use the Java Bridge. I was going to ask
about this.

What's the accepted practice regarding flags? The complete feature will only
build on platforms with a JVM present, so we'll need a compile-time flag. Should
I add a run-time flag too? If so, I guess it makes sense to guard as little as
possible with the compile-time flag, and use the run-time flag where possible?

[jam, 2011-10-21 21:14:23]

On 2011/10/21 16:27:47, Steve Block wrote:
>
http://codereview.chromium.org/8366034/diff/3001/content/browser/tab_contents...
> File content/browser/tab_contents/tab_contents.cc (right):
> 
>
http://codereview.chromium.org/8366034/diff/3001/content/browser/tab_contents...
> content/browser/tab_contents/tab_contents.cc:214:
> java_bridge_dispatcher_host_manager_.reset(
> No, this isn't needed unless we want to use the Java Bridge. I was going to
ask
> about this.
> 
> What's the accepted practice regarding flags? The complete feature will only
> build on platforms with a JVM present, so we'll need a compile-time flag.
Should
> I add a run-time flag too? If so, I guess it makes sense to guard as little as
> possible with the compile-time flag, and use the run-time flag where possible?

I think it would be nice to not compile in this code for platforms that don't
need it, just for code size reasons. see what we do with safe_browsing as an
example

run time flags are usually for experimental features so that they're not on by
default, which isn't the case here.

[Steve Block, 2011-10-24 12:54:36]

> I think it would be nice to not compile in this code for platforms
> that don't need it
Opened http://codereview.chromium.org/8383001 to add ENABLE_JAVA_BRIDGE

[Steve Block, 2011-10-24 23:35:45]

Ready for another look

[jam, 2011-10-25 00:10:12]

lgtm