Refactor BalloonViewHost, removing a lot of duplicate code that crept into no...

Refactor BalloonViewHost, removing a lot of duplicate code that crept into notifications during the port to mac & linux and making behavior more consistent.

BUG=34826
TEST=notifications on each platform.


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

[Andrew T Wilson (Slow), 2010-03-19 23:35:00]

LGTM

http://codereview.chromium.org/1055005/diff/28001/29008
File chrome/browser/notifications/balloon_host.h (right):

http://codereview.chromium.org/1055005/diff/28001/29008#newcode89
chrome/browser/notifications/balloon_host.h:89: virtual RenderWidgetHostView*
render_widget_host_view() const = 0;
This is slightly different from most APIs I've seen for this case. The options
I've seen are:

1) Don't have InitRenderWidgetHostView(), but instead have the platform-specific
implementations of render_widget_host_view() do the creation on the first call
(lazy initialization).
2) Don't make render_widget_host_view() virtual, but instead implement it in the
base class:

virtual RenderWidgetHostView* CreateRenderWidgetHostView();
RenderWidgetHostView* render_widget_host_view() { return
render_widget_host_view_; }

Then, in your init code:

render_widget_host_view_ = CreateRenderWidgetHostView();

The only downside of this is your derived classes have to do a downcast to
convert the result of render_widget_host_view() to the appropriate derived
classes (which you already do on Linux/Win, I believe).

Anyhow, the current implementation is fine also if you don't want to change it.

http://codereview.chromium.org/1055005/diff/28001/29008#newcode91
chrome/browser/notifications/balloon_host.h:91: // Owned pointer to to host for
the renderer process.
nit: "to to"?

[oshima, 2010-03-19 23:44:00]

John,

Can you run this through linux_chromeos try bot to make sure chrome for chromeos
builds file?

thanks,

[John Gregg, 2010-03-20 00:29:41]

thanks oshima, there were a few lines that needed to be updated but i've
uploaded a new patch which does that .

On 2010/03/19 23:44:00, oshima wrote:
> John,
> 
> Can you run this through linux_chromeos try bot to make sure chrome for
chromeos
> builds file?
> 
> thanks,