MacViews: Use native certificate viewer in MacViews browser.

MacViews: Use native certificate viewer in MacViews browser.

We want to use certificate_viewer_mac's version of ShowCertificateViewer
with mac_views_browser, at least for now.

SSLCertificateViewerCocoa depends on ConstrainedWindowMac, which
depends on SingleWebContentsDialogManagerCocoa, which depends on
ConstrainedWindowSheetController. Fix them to no longer depend on
TabStripController.

Refactor GetSheetParentViewForWebContents to virtual
TabDialogs::GetDialogParentView to allow different implementations
between Views and Cocoa on Mac.

Move CreateNativeWebModalManager to a separate file so that
mac_views_browser can exclude it and avoid duplicate definitions.

BUG=425229
Committed: https://crrev.com/978c6d1929384d8f39380c4f4efd84f9e4a2417d
Cr-Commit-Position: refs/heads/master@{#323633}

[Andre, 2015-04-02 04:04:27]

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

[Andre, 2015-04-02 04:04:27]

Trent, WDYT?

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.mm
(right):

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/single_web_contents_dialog_manager_cocoa.mm:38: :
web_contents->GetNativeView();
This is a change when showing a dialog over an app window.
|parent_view| was [web_contents->GetNativeView() superview] which is the
window's contentView, now it's web_contents->GetNativeView().
The result should be the same since they have the same bounds.

[tapted, 2015-04-02 04:35:22]

looks pretty good - i'd go for it. Just one question really

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/cocoa...
File
chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm
(right):

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm:33:
} else {
hm - I see you're just moving this, but this fails the 'no else after return'
style guide test.

should be

  if (devtools_view)
    return [devtools_view convertRect:[devtools_view bounds] toView:nil];
  return [view convertRect:[view bounds] toView:nil];

or

  return devtools_view
      ? [devtools_view convertRect:[devtools_view bounds] toView:nil]
      : [view convertRect:[view bounds] toView:nil];

or

  if (devtools_view)
    view = devtools_view;
  return [view convertRect:[view bounds] toView:nil];

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/tab_dialogs_views.h (right):

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/tab_dialogs_views.h:18: gfx::NativeView
GetDialogParentView() const override;
is there code elsewhere in c/b/ui/views that should be calling this as well?
(e.g. maybe it currently does web_contents_->GetNativeView() and shouldn't)

[Andre, 2015-04-02 17:17:32]

Thanks! This is ready for a more thorough review now.

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/cocoa...
File
chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm
(right):

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/constrained_window/constrained_window_sheet_controller.mm:33:
} else {
On 2015/04/02 04:35:22, tapted wrote:
> hm - I see you're just moving this, but this fails the 'no else after return'
> style guide test.
> 
> should be
> 
>   if (devtools_view)
>     return [devtools_view convertRect:[devtools_view bounds] toView:nil];
>   return [view convertRect:[view bounds] toView:nil];
> 
> or
> 
>   return devtools_view
>       ? [devtools_view convertRect:[devtools_view bounds] toView:nil]
>       : [view convertRect:[view bounds] toView:nil];
> 
> or
> 
>   if (devtools_view)
>     view = devtools_view;
>   return [view convertRect:[view bounds] toView:nil];

Done.

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/tab_dialogs_views.h (right):

https://codereview.chromium.org/1053463002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/tab_dialogs_views.h:18: gfx::NativeView
GetDialogParentView() const override;
On 2015/04/02 04:35:22, tapted wrote:
> is there code elsewhere in c/b/ui/views that should be calling this as well?
> (e.g. maybe it currently does web_contents_->GetNativeView() and shouldn't)

I'm not sure, but TabDialogViews::GetDialogParentView() returns
web_contents_->GetNativeView() anyway so it would be the same.

[Andre, 2015-04-02 17:20:17]

andresantoso@chromium.org changed reviewers:
+ msw@chromium.org, rsesek@chromium.org

[Andre, 2015-04-02 17:20:18]

rsesek@ PTAL for cocoa.
msw@ PTAL for views.

[msw, 2015-04-02 17:35:02]

c/b/ui/views lgtm

[Robert Sesek, 2015-04-02 19:19:47]

LGTM

[Andre, 2015-04-02 20:45:51]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2015-04-02 20:50:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1053463002/60001

[commit-bot: I haz the power, 2015-04-02 21:06:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-02 21:06:25]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Andre, 2015-04-03 00:42:48]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2015-04-03 00:54:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1053463002/60001

[commit-bot: I haz the power, 2015-04-03 05:31:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-03 20:32:41]

Patchset 4 (id:??) landed as
https://crrev.com/978c6d1929384d8f39380c4f4efd84f9e4a2417d
Cr-Commit-Position: refs/heads/master@{#323633}