Pull the file picker code out of TabContents.

Pull the file picker code out of TabContents.

BUG=71097
TEST=hammer on the HTML file selection dialogs. No visible change; no crashes, nothing.

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

[jam, 2011-03-11 17:03:58]

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
File chrome/browser/file_select_helper.cc (right):

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
chrome/browser/file_select_helper.cc:270: if (file_select_helper_.get() == NULL)
nit: usual style is to drop the "== NULL"

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
File chrome/browser/file_select_helper.h (right):

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
chrome/browser/file_select_helper.h:86: class FileSelectObserver : public
TabContentsObserver {
Why do we need a separate class, i.e. why not just make FileSelectHelper a
TabContentsObserver?  It doesn't look like it does anything heavyweight in its
constructor.

[Avi (use Gerrit), 2011-03-11 17:24:41]

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
File chrome/browser/file_select_helper.cc (right):

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
chrome/browser/file_select_helper.cc:270: if (file_select_helper_.get() == NULL)
On 2011/03/11 17:03:58, John Abd-El-Malek wrote:
> nit: usual style is to drop the "== NULL"

Done.

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
File chrome/browser/file_select_helper.h (right):

http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
chrome/browser/file_select_helper.h:86: class FileSelectObserver : public
TabContentsObserver {
On 2011/03/11 17:03:58, John Abd-El-Malek wrote:
> Why do we need a separate class, i.e. why not just make FileSelectHelper a
> TabContentsObserver?

Wish I could, but extensions/extension_host.h uses FileSelectHelper directly.

Perhaps in the future when we switch extensions to use the renamed TabContents,
we can, but for now we can't.

[jam, 2011-03-11 17:32:15]

lgtm

On 2011/03/11 17:24:41, Avi wrote:
>
http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
> File chrome/browser/file_select_helper.cc (right):
> 
>
http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
> chrome/browser/file_select_helper.cc:270: if (file_select_helper_.get() ==
NULL)
> On 2011/03/11 17:03:58, John Abd-El-Malek wrote:
> > nit: usual style is to drop the "== NULL"
> 
> Done.
> 
>
http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
> File chrome/browser/file_select_helper.h (right):
> 
>
http://codereview.chromium.org/6680002/diff/3001/chrome/browser/file_select_h...
> chrome/browser/file_select_helper.h:86: class FileSelectObserver : public
> TabContentsObserver {
> On 2011/03/11 17:03:58, John Abd-El-Malek wrote:
> > Why do we need a separate class, i.e. why not just make FileSelectHelper a
> > TabContentsObserver?
> 
> Wish I could, but extensions/extension_host.h uses FileSelectHelper directly.
> 
> Perhaps in the future when we switch extensions to use the renamed
TabContents,
> we can, but for now we can't.

ah ok, I should have searched first :)

[Avi (use Gerrit), 2011-03-11 17:33:52]

On 2011/03/11 17:32:15, John Abd-El-Malek wrote:
> ah ok, I should have searched first :)

No problem. In fact, that was my brilliant idea at first, and you should have
heard me swear when I saw the compilation issue with ExtensionHost...