Split out some of the RVHDelegate functions into separate sub-classes. To lim...

Split out some of the RVHDelegate functions into separate sub-classes. To limit
the scope, this patch just contains those delegate functions implemented only
by TabContents, plus the favicon functions implemented by the FavIconHelper.
The only changes are re-ordering and moving the functions, and changes in the
way that the functions are called through the new optional delegate.

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

[Erik does not do reviews, 2009-07-07 18:33:05]

overall LGTM - just nits

http://codereview.chromium.org/149239/diff/1/9
File chrome/browser/renderer_host/render_view_host.cc (right):

http://codereview.chromium.org/149239/diff/1/9#newcode1062
Line 1062: favicon_delegate->DidDownloadImage(this, id, image_url, errored,
image);
This method name seems overly generic to me.  I know this has been there for a
while, but while you're in there...

http://codereview.chromium.org/149239/diff/1/9#newcode1329
Line 1329: integration_delegate->GetHistoryListCount(&back_list_count,
&forward_list_count);
80 cols

http://codereview.chromium.org/149239/diff/1/9#newcode1330
Line 1330: Send(new ViewMsg_UpdateBackForwardListCount(
should this message be sent if there isn't a delegate?  it would always be zero
in that case, right?

http://codereview.chromium.org/149239/diff/1/6
File chrome/browser/renderer_host/render_view_host_delegate.h (right):

http://codereview.chromium.org/149239/diff/1/6#newcode171
Line 171: virtual void OnCrashedPlugin(const FilePath& plugin_path) = 0;
I'm not sure that these OnCrashed* methods should be "BrowserIntegration".  I
guess they're there because of the info bar that gets displayed?  I'm pretty
sure we'll need to handle these in extensions, but I'm not convinced the UI
would be the same.  I think they're fine here for now since I don't really have
a better suggestion (a process manager interface?).

http://codereview.chromium.org/149239/diff/1/6#newcode188
Line 188: virtual void DidStartProvisionalLoadForFrame(RenderViewHost*
render_view_host,
there are a bunch of 80 col issues in this file (I won't bother highlighting
each one)

http://codereview.chromium.org/149239/diff/1/8
File chrome/browser/renderer_host/resource_dispatcher_host.cc (right):

http://codereview.chromium.org/149239/diff/1/8#newcode134
Line 134: NOTREACHED();
could you wrap this in {}?  the previous multi-line conditional makes this hard
to read.

http://codereview.chromium.org/149239/diff/1/8#newcode152
Line 152: // The function to call on RenderViewHostDelegate on the UI thread.
update comment

http://codereview.chromium.org/149239/diff/1/3
File chrome/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/149239/diff/1/3#newcode1480
Line 1480: 
I didn't look at these changes closely since I assume they were just moved.  Let
me know if I should review them more.