MacViews: Allow toolkit-views for "Global Error" bubbles.

MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window,

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

Screenshot at http://crbug.com/675496#c1

BUG=675496
Review-Url: https://codereview.chromium.org/2586373003
Cr-Commit-Position: refs/heads/master@{#442503}
Committed: https://chromium.googlesource.com/chromium/src/+/84ce68dbf0aee72a6cfc16b3d50124078836d83c

[tapted, 2016-12-20 06:47:00]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 06:47:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 06:49:09]

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

[commit-bot: I haz the power, 2016-12-20 06:49:10]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tapted, 2017-01-05 05:04:21]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 05:04:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 05:06:11]

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

[commit-bot: I haz the power, 2017-01-05 05:06:12]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[tapted, 2017-01-05 05:55:23]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 05:55:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-01-05 05:57:03]

Patchset #3 (id:40001) has been deleted

[tapted, 2017-01-05 05:57:10]

Patchset #2 (id:20001) has been deleted

[tapted, 2017-01-05 05:57:17]

Patchset #1 (id:1) has been deleted

[tapted, 2017-01-05 05:57:26]

Patchset #2 (id:80001) has been deleted

[commit-bot: I haz the power, 2017-01-05 06:55:44]

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

[commit-bot: I haz the power, 2017-01-05 06:55:45]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tapted, 2017-01-05 07:52:42]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 07:52:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 09:05:22]

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

[commit-bot: I haz the power, 2017-01-05 09:05:23]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-01-05 10:52:37]

Description was changed from

==========
Use toolkit-views for "Global Error" bubbles.

BUG=675496
==========

to

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window.

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

BUG=675496
==========

[tapted, 2017-01-05 10:53:56]

Description was changed from

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window.

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

BUG=675496
==========

to

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window,

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

BUG=675496
==========

[tapted, 2017-01-05 10:53:56]

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

[tapted, 2017-01-05 10:54:58]

Description was changed from

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window,

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

BUG=675496
==========

to

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window,

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

Screenshot at http://crbug.com/675496#c1

BUG=675496
==========

[tapted, 2017-01-05 10:57:05]

Hi Robert, please take a look

[Robert Sesek, 2017-01-05 19:50:08]

> The tricker part is that:
> a) These dialogs are persistent (they do not dismiss when they lose
focus), and

For this issue, could you do something similar to -[BaseBubbleController
registerKeyStateEventTap]. If you receive an event outside of the bridged child
window tell the views widget to close?

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/bubble_anchor_helper_views.mm (right):

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/bubble_anchor_helper_views.mm:107: -
(instancetype)initWithAnchorHelper:(BubbleAnchorHelper*)helper
If you use the block-based -[NSNotificationCenter addObserver:…] method you can
remove this entire ObjC class.

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/global_error_bubble_controller.mm (right):

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/global_error_bubble_controller.mm:35: NSWindow*
parentWindow = browser->window()->GetNativeWindow();
naming: parent_window

[tapted, 2017-01-06 00:12:51]

The CQ bit was checked by tapted@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 00:13:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-01-06 00:14:05]

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/bubble_anchor_helper_views.mm (right):

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/bubble_anchor_helper_views.mm:107: -
(instancetype)initWithAnchorHelper:(BubbleAnchorHelper*)helper
On 2017/01/05 19:50:08, Robert Sesek wrote:
> If you use the block-based -[NSNotificationCenter addObserver:…] method you
can
> remove this entire ObjC class.

Oooh neato. Done.

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
File chrome/browser/ui/cocoa/global_error_bubble_controller.mm (right):

https://codereview.chromium.org/2586373003/diff/180001/chrome/browser/ui/coco...
chrome/browser/ui/cocoa/global_error_bubble_controller.mm:35: NSWindow*
parentWindow = browser->window()->GetNativeWindow();
On 2017/01/05 19:50:08, Robert Sesek wrote:
> naming: parent_window

Done.

[commit-bot: I haz the power, 2017-01-06 01:26:52]

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

[commit-bot: I haz the power, 2017-01-06 01:26:53]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-01-06 01:30:08]

Whoops - forgot this bit. I had made a CL and everything:

On 2017/01/05 19:50:08, Robert Sesek wrote:
> > The tricker part is that:
> > a) These dialogs are persistent (they do not dismiss when they lose
> focus), and
> 
> For this issue, could you do something similar to -[BaseBubbleController
> registerKeyStateEventTap]. If you receive an event outside of the bridged
child
> window tell the views widget to close?

We actually want the bubble to be persistent :). It's actually a bug on
Mac/Cocoa that it is not. The bubble is a pain to invoke, but I made
https://codereview.chromium.org/2610923009/ in case you want to try it on Cocoa.
The Cocoa bubble dismisses when clicking away, but it should not.

The views bubble does:

void GlobalErrorBubbleView::Init() {
  ...
  // These bubbles show at times where activation is sporadic (like at startup,
  // or a new window opening). Make sure the bubble doesn't disappear before the
  // user sees it, if the bubble needs to be acknowledged.
  set_close_on_deactivate(error_->ShouldCloseOnDeactivate());
}

[Robert Sesek, 2017-01-06 22:28:59]

On 2017/01/06 01:30:08, tapted wrote:
> Whoops - forgot this bit. I had made a CL and everything:
> 
> On 2017/01/05 19:50:08, Robert Sesek wrote:
> > > The tricker part is that:
> > > a) These dialogs are persistent (they do not dismiss when they lose
> > focus), and
> > 
> > For this issue, could you do something similar to -[BaseBubbleController
> > registerKeyStateEventTap]. If you receive an event outside of the bridged
> child
> > window tell the views widget to close?
> 
> We actually want the bubble to be persistent :). It's actually a bug on
> Mac/Cocoa that it is not. The bubble is a pain to invoke, but I made
> https://codereview.chromium.org/2610923009/ in case you want to try it on
Cocoa.
> The Cocoa bubble dismisses when clicking away, but it should not.
> 
> The views bubble does:
> 
> void GlobalErrorBubbleView::Init() {
>   ...
>   // These bubbles show at times where activation is sporadic (like at
startup,
>   // or a new window opening). Make sure the bubble doesn't disappear before
the
>   // user sees it, if the bubble needs to be acknowledged.
>   set_close_on_deactivate(error_->ShouldCloseOnDeactivate());
> }

Ah, I'm not sure how I feel about this type of window-modal dialog. It seems a
little non-Mac-y since I'd expect that kind of thing to be a sheet. But LGTM
either way.

[tapted, 2017-01-09 21:43:44]

On 2017/01/06 22:28:59, Robert Sesek wrote:
> On 2017/01/06 01:30:08, tapted wrote:
> > Whoops - forgot this bit. I had made a CL and everything:
> > 
> > On 2017/01/05 19:50:08, Robert Sesek wrote:
> > > > The tricker part is that:
> > > > a) These dialogs are persistent (they do not dismiss when they lose
> > > focus), and
> > > 
> > > For this issue, could you do something similar to -[BaseBubbleController
> > > registerKeyStateEventTap]. If you receive an event outside of the bridged
> > child
> > > window tell the views widget to close?
> > 
> > We actually want the bubble to be persistent :). It's actually a bug on
> > Mac/Cocoa that it is not. The bubble is a pain to invoke, but I made
> > https://codereview.chromium.org/2610923009/ in case you want to try it on
> Cocoa.
> > The Cocoa bubble dismisses when clicking away, but it should not.
> > 
> > The views bubble does:
> > 
> > void GlobalErrorBubbleView::Init() {
> >   ...
> >   // These bubbles show at times where activation is sporadic (like at
> startup,
> >   // or a new window opening). Make sure the bubble doesn't disappear before
> the
> >   // user sees it, if the bubble needs to be acknowledged.
> >   set_close_on_deactivate(error_->ShouldCloseOnDeactivate());
> > }
> 
> Ah, I'm not sure how I feel about this type of window-modal dialog. It seems a
> little non-Mac-y since I'd expect that kind of thing to be a sheet. But LGTM
> either way.

Yeah.. I agree these are a little weird. There is precedent in Permissions
Dialogs (although they disappear temporarily when you switch tabs), but also the
"Developer Mode Extensions" dialog. There's a bunch of tradeoffs in ensuring the
user notices the dialog (e.g. because something has been disabled, or may be
malicious), while not getting completely in the way, but also ensuring there's
no feeling of "oh crap that dialog disappeared - how did I get it back"..

[tapted, 2017-01-09 21:43:58]

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

[tapted, 2017-01-09 21:44:31]

+msw for c/b/ui/views/global_error_bubble_view.* OWNERS - please take a look

[msw, 2017-01-10 00:44:31]

c/b/ui/views/global_error_bubble_view.* lgtm

[tapted, 2017-01-10 02:40:39]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2017-01-10 02:41:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 05:53:50]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1484016039438620,
"parent_rev": "ee16fc4f187bb47b7a2a0535c59ae9feb152a169", "commit_rev":
"84ce68dbf0aee72a6cfc16b3d50124078836d83c"}

[commit-bot: I haz the power, 2017-01-10 05:54:22]

Description was changed from

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window,

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

Screenshot at http://crbug.com/675496#c1

BUG=675496
==========

to

==========
MacViews: Allow toolkit-views for "Global Error" bubbles.

The views dialogs will be chosen when run with the --secondary-ui-md
flag.

When the parent is a Cocoa browser window, an anchor point is chosen
rather than an anchor view, and the GlobalErrorBubbleView constructor is
updated to use this point when the anchor view is null.

The tricker part is that:
a) These dialogs are persistent (they do not dismiss when they lose
focus), and
b) The dialogs appear on the right side of the window,

This means they're more prone to "falling off" their anchor point when
resizing the browser window. For this, add a "BubbleAnchorHelper"
framework which is enabled by passing a visible toolkit-views bubble to
a KeepBubbleAnchored() function. The framework captures the anchor
position and ensures it is maintained when changes to the parent
NSWindow are observed.

Screenshot at http://crbug.com/675496#c1

BUG=675496

Review-Url: https://codereview.chromium.org/2586373003
Cr-Commit-Position: refs/heads/master@{#442503}
Committed:
https://chromium.googlesource.com/chromium/src/+/84ce68dbf0aee72a6cfc16b3d501...
==========

[commit-bot: I haz the power, 2017-01-10 05:54:24]

Committed patchset #7 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/84ce68dbf0aee72a6cfc16b3d501...