Input file type now supported in extension popups.

Input file type now supported in extension popups.
To do this, this CL generalize the TabContentsFileSelectHelper (renamed FileSelectHelper) so it is associated with a RenderViewHost rather than a TabContents.
This allows the extension popups which don't use a TabContents to use it.

As part of that, I also moved GetTopLevelNativeWindow() from TabContentsView to TabContent, as it can be implemented in a non-platform specific way.

BUG=28829
TEST=Make sure you can still open file on web pages (such as http://www.cs.tut.fi/~jkorpela/forms/file.html.
     Create an extension with a popup that contains an input file tag. Make sure it does open a file dialog and lets you choose a file.

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

[Jay Civelli, 2010-08-25 21:37:55]

Ben, could you look at the changes to the tab_contents_* file and let me know if
you are OK with moving GetTopLevelNativeWindow from TabContentsView to
TabContents.

John, could you look at the other files?

[John Gregg, 2010-08-26 18:20:13]

One concern that comes to mind is that the FileSelectHelper is currently
assuming it will only be handling on file selection at a time.  When tied to a
single RenderViewHost this is guaranteed because the renderer queues up those
requests.  

Maybe that still applies, but when you pass that host in to RunFileChooser it's
not (obviously) guaranteed anymore.  You might want to add logic in
FileSelectHelper::RunFileChooser to cancel anything that's going on if the host
has changed.

[John Gregg, 2010-08-26 18:31:35]

On second thought, you have a DCHECK for this.  So if you're convinced that
extensions won't ever trigger that, I'm fine with it.

Only other comment would be if you could use a rename for the
file_select_helper.cc file the same way you do for the .h file so the diff can
be preserved.

[Jay Civelli, 2010-08-26 22:47:53]

On 2010/08/26 18:31:35, John Gregg wrote:
> On second thought, you have a DCHECK for this.  So if you're convinced that
> extensions won't ever trigger that, I'm fine with it.
Yes, the extension should not trigger multiple file selections at a time since
the popup is backed by a single RV, so it should be similar to the TabContents
case.

> Only other comment would be if you could use a rename for the
> file_select_helper.cc file the same way you do for the .h file so the diff can
> be preserved.
I could swear I did a git mv for both files. Not sure why it did not work as
expected for the .cc I tried on my Linux box (patched and recreated the CL) but
it still the same. Not sure what's going on.

[John Gregg, 2010-08-30 18:57:59]

On 2010/08/26 22:47:53, Jay Civelli wrote:
> On 2010/08/26 18:31:35, John Gregg wrote:
> > On second thought, you have a DCHECK for this.  So if you're convinced that
> > extensions won't ever trigger that, I'm fine with it.
> Yes, the extension should not trigger multiple file selections at a time since
> the popup is backed by a single RV, so it should be similar to the TabContents
> case.
> 
> > Only other comment would be if you could use a rename for the
> > file_select_helper.cc file the same way you do for the .h file so the diff
can
> > be preserved.
> I could swear I did a git mv for both files. Not sure why it did not work as
> expected for the .cc I tried on my Linux box (patched and recreated the CL)
but
> it still the same. Not sure what's going on.

Okay, that's fine and LGTM.  

Regarding the rename, I have a fix to put in on the FileSelectHelper, which is
that directory_lister_results_ should be cleared at the end of OnListDone. 
Since it looks risky I don't want to check it in before you, but can you just
add it to your patch?  Otherwise I'll wait for this to land.

[Jay Civelli, 2010-08-30 21:49:22]

I added the change you mentioned and uploaded a new CL.
Let me know if that is not right.

Jay

On Mon, Aug 30, 2010 at 11:57 AM,  <johnnyg@chromium.org> wrote:
> On 2010/08/26 22:47:53, Jay Civelli wrote:
>>
>> On 2010/08/26 18:31:35, John Gregg wrote:
>> > On second thought, you have a DCHECK for this.  So if you're convinced
>> > that
>> > extensions won't ever trigger that, I'm fine with it.
>> Yes, the extension should not trigger multiple file selections at a time
>> since
>> the popup is backed by a single RV, so it should be similar to the
>> TabContents
>> case.
>
>> > Only other comment would be if you could use a rename for the
>> > file_select_helper.cc file the same way you do for the .h file so the
>> > diff
>
> can
>>
>> > be preserved.
>> I could swear I did a git mv for both files. Not sure why it did not work
>> as
>> expected for the .cc I tried on my Linux box (patched and recreated the
>> CL)
>
> but
>>
>> it still the same. Not sure what's going on.
>
> Okay, that's fine and LGTM.
>
> Regarding the rename, I have a fix to put in on the FileSelectHelper, which
> is
> that directory_lister_results_ should be cleared at the end of OnListDone.
> Since it looks risky I don't want to check it in before you, but can you
> just
> add it to your patch?  Otherwise I'll wait for this to land.
>
> http://codereview.chromium.org/3209002/show
>

[johnnyg, 2010-08-30 21:51:30]

It looks right.  Thanks!

[Jay Civelli, 2010-09-01 21:11:15]

Hey Ben,
Could you look at the changes to the tab_contents_* files and let me know if
you are OK with moving GetTopLevelNativeWindow from TabContentsView to
TabContents.

Thanks!

Jay

[Jay Civelli, 2010-09-09 17:16:02]

Scott,
Could you look at the changes to the tab_contents_* files and let me know if
you think it is OK to move GetTopLevelNativeWindow from TabContentsView to
TabContents.

Thanks!

Jay

[sky, 2010-09-09 19:22:48]

Ben says he's on it.

  -Scott

On Thu, Sep 9, 2010 at 10:16 AM,  <jcivelli@chromium.org> wrote:
> Scott,
> Could you look at the changes to the tab_contents_* files and let me know if
> you think it is OK to move GetTopLevelNativeWindow from TabContentsView to
> TabContents.
>
> Thanks!
>
> Jay
>
> http://codereview.chromium.org/3209002/show
>

[Jay Civelli, 2010-09-09 23:16:35]

OK, I moved back GetTopLevelWindow to the TabContentsView.

[Ben Goodger (Google), 2010-09-10 01:01:36]

On 2010/09/09 23:16:35, Jay Civelli wrote:
> OK, I moved back GetTopLevelWindow to the TabContentsView.

LG