MacViews: add extension install dialog to MacViewsWebUIDialogs

MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657
Committed: https://crrev.com/0acb4542a40e8d4c053302da1db1c39e067a3a93
Cr-Commit-Position: refs/heads/master@{#393854}

[Elly Fong-Jones, 2016-05-12 19:20:25]

Description was changed from

==========
MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657
==========

to

==========
MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657
==========

[Elly Fong-Jones, 2016-05-12 19:20:25]

ellyjones@chromium.org changed reviewers:
+ tapted@chromium.org

[Elly Fong-Jones, 2016-05-12 19:21:15]

ellyjones@chromium.org changed reviewers:
+ sky@chromium.org

[Elly Fong-Jones, 2016-05-12 19:21:16]

tapted: ptal? re-upload of reverted https://codereview.chromium.org/1918793004
with the mac_views_browser=1 build fixed via a shim.

sky: ptal? :)

[tapted, 2016-05-13 07:42:08]

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_install_prompt.h (right):

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_install_prompt.h:267: static
ShowDialogCallback GetViewsShowDialogCallback();
I think it would be nicer to expose this on all platforms -  see below for how
that would work.

(Ideally it would move to
chrome/browser/ui/views/extensions/extension_install_dialog_view.h but DEPS will
complain if extension_install_dialog_controller.mm tries to include that) -

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/extension_install_dialog_view_shim.cc
(right):

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/extension_install_dialog_view_shim.cc:9:
ExtensionInstallPrompt::GetDefaultShowDialogCallback() {
Can this go in chrome/browser/ui/views/browser_dialog_views.cc ? There's a
comment in that file:

// This file provides definitions of desktop browser dialog-creation methods for
// all toolkit-views platforms other than Mac. It also provides the definitions
// on Mac when mac_views_browser=1

I think that's the right fit for this. It's fine to #include
chrome/browser/ui/views/extensions/extension_install_dialog_view.h there, and I
think Views \implies{} extensions, so there's no need to wrap it in #if
defined(ENABLE_EXTENSIONS)

https://codereview.chromium.org/1977593002/diff/1/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/1977593002/diff/1/chrome/chrome_browser_ui.gy...
chrome/chrome_browser_ui.gypi:2480:
'browser/ui/views/extensions/extension_install_dialog_view.cc',
for now this should be fine to just move to chrome_browser_ui_views_sources.
There's already, e.g.,
browser/ui/views/extensions/extension_keybinding_registry_views there.

[tapted, 2016-05-13 07:54:57]

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/extension_install_dialog_view_shim.cc
(right):

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/extension_install_dialog_view_shim.cc:9:
ExtensionInstallPrompt::GetDefaultShowDialogCallback() {
On 2016/05/13 07:42:08, tapted wrote:
> t's fine to #include
> chrome/browser/ui/views/extensions/extension_install_dialog_view.h [in
chrome/browser/ui/views/browser_dialog_views.cc]

oops ignore just this part of the comment -
"chrome/browser/extensions/extension_install_prompt.h" is the one you'll want,
so that the Cocoa file can get at it too.

[Elly Fong-Jones, 2016-05-13 21:41:52]

tapted: ptal? :)

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/extensions/e...
File chrome/browser/extensions/extension_install_prompt.h (right):

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/extensions/e...
chrome/browser/extensions/extension_install_prompt.h:267: static
ShowDialogCallback GetViewsShowDialogCallback();
On 2016/05/13 07:42:08, tapted wrote:
> I think it would be nicer to expose this on all platforms -  see below for how
> that would work.
> 
> (Ideally it would move to
> chrome/browser/ui/views/extensions/extension_install_dialog_view.h but DEPS
will
> complain if extension_install_dialog_controller.mm tries to include that) -

Done.

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/extension_install_dialog_view_shim.cc
(right):

https://codereview.chromium.org/1977593002/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/extension_install_dialog_view_shim.cc:9:
ExtensionInstallPrompt::GetDefaultShowDialogCallback() {
On 2016/05/13 07:54:57, tapted wrote:
> On 2016/05/13 07:42:08, tapted wrote:
> > t's fine to #include
> > chrome/browser/ui/views/extensions/extension_install_dialog_view.h [in
> chrome/browser/ui/views/browser_dialog_views.cc]
> 
> oops ignore just this part of the comment -
> "chrome/browser/extensions/extension_install_prompt.h" is the one you'll want,
> so that the Cocoa file can get at it too.

Done.

https://codereview.chromium.org/1977593002/diff/1/chrome/chrome_browser_ui.gypi
File chrome/chrome_browser_ui.gypi (right):

https://codereview.chromium.org/1977593002/diff/1/chrome/chrome_browser_ui.gy...
chrome/chrome_browser_ui.gypi:2480:
'browser/ui/views/extensions/extension_install_dialog_view.cc',
On 2016/05/13 07:42:08, tapted wrote:
> for now this should be fine to just move to chrome_browser_ui_views_sources.
> There's already, e.g.,
> browser/ui/views/extensions/extension_keybinding_registry_views there.

Done.

[tapted, 2016-05-13 22:14:34]

lgtm with build.gn diff gone

https://codereview.chromium.org/1977593002/diff/20001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/1977593002/diff/20001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:293:
rebase_path(gypi_values.chrome_browser_ui_views_extensions_sources,
this bit is no longer needed

[sky, 2016-05-13 23:33:20]

LGTM

[Elly Fong-Jones, 2016-05-16 16:17:44]

https://codereview.chromium.org/1977593002/diff/20001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/1977593002/diff/20001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:293:
rebase_path(gypi_values.chrome_browser_ui_views_extensions_sources,
On 2016/05/13 22:14:34, tapted wrote:
> this bit is no longer needed

Done.

[Elly Fong-Jones, 2016-05-16 16:17:48]

The CQ bit was checked by ellyjones@chromium.org

[Elly Fong-Jones, 2016-05-16 16:17:48]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1977593002/#ps40001
(title: "Remove BUILD.gn changes")

[commit-bot: I haz the power, 2016-05-16 16:17:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1977593002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1977593002/40001

[commit-bot: I haz the power, 2016-05-16 17:20:40]

Description was changed from

==========
MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657
==========

to

==========
MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657
==========

[commit-bot: I haz the power, 2016-05-16 17:20:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-16 17:22:04]

Description was changed from

==========
MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657
==========

to

==========
MacViews: add extension install dialog to MacViewsWebUIDialogs

This dialog is now behind the MacViewsWebUIDialogs feature flag.

BUG=605657

Committed: https://crrev.com/0acb4542a40e8d4c053302da1db1c39e067a3a93
Cr-Commit-Position: refs/heads/master@{#393854}
==========

[commit-bot: I haz the power, 2016-05-16 17:22:07]

Patchset 3 (id:??) landed as
https://crrev.com/0acb4542a40e8d4c053302da1db1c39e067a3a93
Cr-Commit-Position: refs/heads/master@{#393854}