Bring back the file dialogs for chrome AURA.

Bring back the file dialogs for chrome AURA. 

The AURA specific changes have been isolated in select_file_dialog_win.cc where we retrieve the
top level window handle from the AURA root window.

As per a discussion with Ben fixed the layering violation where in ui\base was depending on
gfx::NativeView/NativeWindow which led to a layering violation on AURA. This was via ui\base\dialogs
Moved the files out of ui\base\dialogs into a new gyp shell_dialogs.gyp which lives in ui\shell_dialogs.

The relevant code in chrome browser now depends on this shell_dialogs target created by shell_dialogs.gyp

BUG=151718

[Ben Goodger (Google), 2013-01-14 23:23:23]

https://codereview.chromium.org/11878031/diff/1/chrome/browser/download/save_...
File chrome/browser/download/save_package_file_picker.cc (right):

https://codereview.chromium.org/11878031/diff/1/chrome/browser/download/save_...
chrome/browser/download/save_package_file_picker.cc:170: parent_view =
reinterpret_cast<gfx::NativeWindow>(
This is wrong but probably compiles and "works" because the HWND is just an
integer value.

gfx::NativeWindow typedefs to aura::Window* in USE_AURA builds.

https://codereview.chromium.org/11878031/diff/1/ui/base/dialogs/select_file_d...
File ui/base/dialogs/select_file_dialog_win.cc (right):

https://codereview.chromium.org/11878031/diff/1/ui/base/dialogs/select_file_d...
ui/base/dialogs/select_file_dialog_win.cc:536: HWND owner =
reinterpret_cast<HWND>(owning_window);
You should instead do the ->GetRootWindow()->GetAcceleratedWidget() here instead
of reinterpret-casting.

[ananta, 2013-01-14 23:26:32]

https://codereview.chromium.org/11878031/diff/1/chrome/browser/download/save_...
File chrome/browser/download/save_package_file_picker.cc (right):

https://codereview.chromium.org/11878031/diff/1/chrome/browser/download/save_...
chrome/browser/download/save_package_file_picker.cc:170: parent_view =
reinterpret_cast<gfx::NativeWindow>(
On 2013/01/14 23:23:23, Ben Goodger (Google) wrote:
> This is wrong but probably compiles and "works" because the HWND is just an
> integer value.
> 
> gfx::NativeWindow typedefs to aura::Window* in USE_AURA builds. 

Yes. We cannot use GetAcceleratedWidget from select_file_dialog_win.cc as ui
cannot depend on aura due to
cyclic dependencies.

https://codereview.chromium.org/11878031/diff/1/ui/base/dialogs/select_file_d...
File ui/base/dialogs/select_file_dialog_win.cc (right):

https://codereview.chromium.org/11878031/diff/1/ui/base/dialogs/select_file_d...
ui/base/dialogs/select_file_dialog_win.cc:536: HWND owner =
reinterpret_cast<HWND>(owning_window);
On 2013/01/14 23:23:23, Ben Goodger (Google) wrote:
> You should instead do the ->GetRootWindow()->GetAcceleratedWidget() here
instead
> of reinterpret-casting.

I started out with this originally. ui cannot depend on aura which causes cyclic
dependencies.

[ananta, 2013-01-15 03:19:21]

As per our discussion moved ui\base\dialogs to ui\shell_folders with a new gyp
file shell_folders.gyp.

PTAL.

[Ben Goodger (Google), 2013-01-15 04:44:43]

I am going to talk to erg about the linux includes in ui/base tomorrow morning.

https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h
File ui/base/linux_ui.h (right):

https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h#newcode10
ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_dialog.h"
oof. this is a bad dependency. shell_dialogs should depend on ui/base, but
ui/base shouldn't depend on shell_dialogs.

https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h#newcode49
ui/base/linux_ui.h:49: virtual SelectFileDialog* CreateSelectFileDialog(
it's this one.

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/base_shel...
File ui/shell_dialogs/base_shell_dialog.h (right):

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/base_shel...
ui/shell_dialogs/base_shell_dialog.h:5: #ifndef
UI_BASE_DIALOGS_BASE_SHELL_DIALOG_H_
update these too.

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/select_fi...
File ui/shell_dialogs/select_file_dialog_win.cc (right):

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/select_fi...
ui/shell_dialogs/select_file_dialog_win.cc:33: #if defined(OS_WIN) &&
defined(USE_AURA)
OS_WIN is implied by the filename.

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/shell_dia...
File ui/shell_dialogs/shell_dialogs.gyp (right):

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/shell_dia...
ui/shell_dialogs/shell_dialogs.gyp:12: 'type': 'static_library',
<(component)

[Ben Goodger (Google), 2013-01-15 19:07:13]

I am adding elliot to this cl as he knows the linux stuff better than I. Elliot
there's a circular/bad dependency here in this cl in its current form. We need
to figure out a correct location/dependency diagram for this code.

[ananta, 2013-01-15 19:32:28]

https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h
File ui/base/linux_ui.h (right):

https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h#newcode10
ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_dialog.h"
On 2013/01/15 04:44:43, Ben Goodger (Google) wrote:
> oof. this is a bad dependency. shell_dialogs should depend on ui/base, but
> ui/base shouldn't depend on shell_dialogs.
Will look into this.

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/base_shel...
File ui/shell_dialogs/base_shell_dialog.h (right):

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/base_shel...
ui/shell_dialogs/base_shell_dialog.h:5: #ifndef
UI_BASE_DIALOGS_BASE_SHELL_DIALOG_H_
On 2013/01/15 04:44:43, Ben Goodger (Google) wrote:
> update these too.

Done.

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/select_fi...
File ui/shell_dialogs/select_file_dialog_win.cc (right):

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/select_fi...
ui/shell_dialogs/select_file_dialog_win.cc:33: #if defined(OS_WIN) &&
defined(USE_AURA)
On 2013/01/15 04:44:43, Ben Goodger (Google) wrote:
> OS_WIN is implied by the filename.

Done.

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/shell_dia...
File ui/shell_dialogs/shell_dialogs.gyp (right):

https://codereview.chromium.org/11878031/diff/6012/ui/shell_dialogs/shell_dia...
ui/shell_dialogs/shell_dialogs.gyp:12: 'type': 'static_library',
On 2013/01/15 04:44:43, Ben Goodger (Google) wrote:
> <(component)

Done.

[Ben Goodger (Google), 2013-01-15 22:41:47]

On Tue, Jan 15, 2013 at 11:32 AM, <ananta@chromium.org> wrote:

>
>
https://codereview.chromium.**org/11878031/diff/6012/ui/**base/linux_ui.h<htt...
> File ui/base/linux_ui.h (right):
>
> https://codereview.chromium.**org/11878031/diff/6012/ui/**
>
base/linux_ui.h#newcode10<https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h#newcode10>
> ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_**dialog.h"
> On 2013/01/15 04:44:43, Ben Goodger (Google) wrote:
>
>> oof. this is a bad dependency. shell_dialogs should depend on ui/base,
>>
> but
>
>> ui/base shouldn't depend on shell_dialogs.
>>
> Will look into this.


Elliot should help you detangle this part.

-Ben

[ananta, 2013-01-15 23:17:48]

On 2013/01/15 22:41:47, Ben Goodger (Google) wrote:
> On Tue, Jan 15, 2013 at 11:32 AM, <mailto:ananta@chromium.org> wrote:
> 
> >
> >
>
https://codereview.chromium.**org/11878031/diff/6012/ui/**base/linux_ui.h%3Ch...>
> > File ui/base/linux_ui.h (right):
> >
> > https://codereview.chromium.**org/11878031/diff/6012/ui/**
> >
>
base/linux_ui.h#newcode10<https://codereview.chromium.org/11878031/diff/6012/ui/base/linux_ui.h#newcode10>
> > ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_**dialog.h"
> > On 2013/01/15 04:44:43, Ben Goodger (Google) wrote:
> >
> >> oof. this is a bad dependency. shell_dialogs should depend on ui/base,
> >>
> > but
> >
> >> ui/base shouldn't depend on shell_dialogs.
> >>
> > Will look into this.
> 
> 
> Elliot should help you detangle this part.
> 
> -Ben

Based on discussion with Elliot, we decided that for now we could have a
subclass of the LinuxUI class in ui\shell_dialogs. The CreateSelectFileDialog
member from LinuxUI will move to this class. 
The Gtk2UI class which is defined in chrome/browser/ui/libgtk2ui/gtk2_ui.h would
subclass the new class and override the CreateSelectFileDialog function as
before.

[Ben Goodger (Google), 2013-01-15 23:18:58]

ok let me know when the cl is updated.

[ananta, 2013-01-15 23:23:20]

On 2013/01/15 23:18:58, Ben Goodger (Google) wrote:
> ok let me know when the cl is updated.

The CL has just been updated.

Thanks
Ananta

[Ben Goodger (Google), 2013-01-15 23:26:42]

lgtm

https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h
File ui/base/linux_ui.h (right):

https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h#newcode10
ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_dialog.h"
you don't need this anymore then right?

[Elliot Glaysher, 2013-01-15 23:28:52]

I suppose splitting LinuxUI into a derived interface works, even if I'm unhappy
with the casting introduced to SFD.

lgtm

https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h
File ui/base/linux_ui.h (right):

https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h#newcode10
ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_dialog.h"
Is this permanent? I'd assume dependencies are the other way around.

https://codereview.chromium.org/11878031/diff/60001/ui/shell_dialogs/select_f...
File ui/shell_dialogs/select_file_dialog.h (right):

https://codereview.chromium.org/11878031/diff/60001/ui/shell_dialogs/select_f...
ui/shell_dialogs/select_file_dialog.h:205: class SHELL_DIALOGS_EXPORT
LinuxUIShellDialog : public ui::LinuxUI {
Should this be in its own header instead?

[ananta, 2013-01-15 23:43:42]

https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h
File ui/base/linux_ui.h (right):

https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h#newcode10
ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_dialog.h"
On 2013/01/15 23:26:42, Ben Goodger (Google) wrote:
> you don't need this anymore then right?

Removed

https://codereview.chromium.org/11878031/diff/60001/ui/shell_dialogs/select_f...
File ui/shell_dialogs/select_file_dialog.h (right):

https://codereview.chromium.org/11878031/diff/60001/ui/shell_dialogs/select_f...
ui/shell_dialogs/select_file_dialog.h:205: class SHELL_DIALOGS_EXPORT
LinuxUIShellDialog : public ui::LinuxUI {
On 2013/01/15 23:28:52, Elliot Glaysher wrote:
> Should this be in its own header instead?
Done: Moved this to a new file linux_ui_shell_dialog.h

[ananta, 2013-01-16 23:53:22]

On 2013/01/15 23:43:42, ananta wrote:
> https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h
> File ui/base/linux_ui.h (right):
> 
>
https://codereview.chromium.org/11878031/diff/60001/ui/base/linux_ui.h#newcode10
> ui/base/linux_ui.h:10: #include "ui/shell_dialogs/select_file_dialog.h"
> On 2013/01/15 23:26:42, Ben Goodger (Google) wrote:
> > you don't need this anymore then right?
> 
> Removed
> 
>
https://codereview.chromium.org/11878031/diff/60001/ui/shell_dialogs/select_f...
> File ui/shell_dialogs/select_file_dialog.h (right):
> 
>
https://codereview.chromium.org/11878031/diff/60001/ui/shell_dialogs/select_f...
> ui/shell_dialogs/select_file_dialog.h:205: class SHELL_DIALOGS_EXPORT
> LinuxUIShellDialog : public ui::LinuxUI {
> On 2013/01/15 23:28:52, Elliot Glaysher wrote:
> > Should this be in its own header instead?
> Done: Moved this to a new file linux_ui_shell_dialog.h

Please ignore this change for now. Due to problems with running tryjobs off this
change, I will upload a new change. 

Thanks
Ananta