MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151
Committed: https://crrev.com/d6725376ebed11cd889ede0c146fdda81c84ee56
Cr-Commit-Position: refs/heads/master@{#434911}

[tapted, 2016-11-29 03:58:16]

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

[tapted, 2016-11-29 03:58:27]

Description was changed from

==========
MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151
==========

to

==========
MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151
==========

[tapted, 2016-11-29 03:58:27]

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

[commit-bot: I haz the power, 2016-11-29 03:58:31]

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

[tapted, 2016-11-29 03:59:02]

Hi Karan - please take a look

[commit-bot: I haz the power, 2016-11-29 04:40:13]

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

[commit-bot: I haz the power, 2016-11-29 04:40:14]

Dry run: This issue passed the CQ dry run.

[karandeepb, 2016-11-29 05:28:43]

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:695: // themselves a Widget (those are
skipped by this logic).
It's not clear to me what this comment means- "Note we assume we don't care
about children of Widgets that are not themselves a Widget (those are skipped by
this logic)."

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:706: // children of its own on Mac).
Should we not do the same for this case?

[tapted, 2016-11-29 05:50:37]

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

[commit-bot: I haz the power, 2016-11-29 05:50:48]

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

[tapted, 2016-11-29 05:51:42]

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:695: // themselves a Widget (those are
skipped by this logic).
On 2016/11/29 05:28:42, karandeepb wrote:
> It's not clear to me what this comment means- "Note we assume we don't care
> about children of Widgets that are not themselves a Widget (those are skipped
by
> this logic)."

Moved it down to the other for loop and reworded.

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:706: // children of its own on Mac).
On 2016/11/29 05:28:42, karandeepb wrote:
> Should we not do the same for this case?

Done.

[karandeepb, 2016-11-29 06:00:20]

lgtm

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:695: // themselves a Widget (those are
skipped by this logic).
On 2016/11/29 05:51:42, tapted wrote:
> On 2016/11/29 05:28:42, karandeepb wrote:
> > It's not clear to me what this comment means- "Note we assume we don't care
> > about children of Widgets that are not themselves a Widget (those are
skipped
> by
> > this logic)."
> 
> Moved it down to the other for loop and reworded.

Acknowledged.

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:706: // children of its own on Mac).
On 2016/11/29 05:51:42, tapted wrote:
> On 2016/11/29 05:28:42, karandeepb wrote:
> > Should we not do the same for this case?
> 
> Done.

Actually I meant iterating over the child windows of [[native_view window]
childWindows] for this case as well, not the other way round. But yeah it's not
clear if for this case, [[native_view window] childWindows] can be considered to
be in the hierarchy of native_view, probably not.

[tapted, 2016-11-29 06:06:11]

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
File ui/views/widget/native_widget_mac.mm (right):

https://codereview.chromium.org/2535163002/diff/1/ui/views/widget/native_widg...
ui/views/widget/native_widget_mac.mm:706: // children of its own on Mac).
On 2016/11/29 06:00:20, karandeepb wrote:
> On 2016/11/29 05:51:42, tapted wrote:
> > On 2016/11/29 05:28:42, karandeepb wrote:
> > > Should we not do the same for this case?
> > 
> > Done.
> 
> Actually I meant iterating over the child windows of [[native_view window]
> childWindows] for this case as well, not the other way round. But yeah it's
not
> clear if for this case, [[native_view window] childWindows] can be considered
to
> be in the hierarchy of native_view, probably not. 

Ah, yep. GetBridgeForNativeWindow uses the NSWindow delegate -- there can be
only one per window, so it might be confusing. We can always revisit if we find
a use-case for it though.

[tapted, 2016-11-29 06:06:17]

The CQ bit was unchecked by tapted@chromium.org

[tapted, 2016-11-29 06:06:24]

The CQ bit was checked by tapted@chromium.org

[commit-bot: I haz the power, 2016-11-29 06:06:37]

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-11-29 06:28:45]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480399584765880,
"parent_rev": "14cd05cef8765cb2bb23accd7c97f5d431bad6e7", "commit_rev":
"9c75ef743101ac05374cf1196a1c5269ccde7cf1"}

[commit-bot: I haz the power, 2016-11-29 06:29:37]

Description was changed from

==========
MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151
==========

to

==========
MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151
==========

[commit-bot: I haz the power, 2016-11-29 06:29:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-29 06:32:45]

Description was changed from

==========
MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151
==========

to

==========
MacViews: Fix Widget::GetAllChildWidgets(gfx::NativeView) for non-Widgets.

Currently GetAllChildWidgets only works for Widgets, but the API doesn't
require that. Passing a non-Widget allows the Widgets parented off a
Cocoa browser window to be easily obtained. This will help with testing
of browser dialogs.

BUG=654151

Committed: https://crrev.com/d6725376ebed11cd889ede0c146fdda81c84ee56
Cr-Commit-Position: refs/heads/master@{#434911}
==========

[commit-bot: I haz the power, 2016-11-29 06:32:47]

Patchset 2 (id:??) landed as
https://crrev.com/d6725376ebed11cd889ede0c146fdda81c84ee56
Cr-Commit-Position: refs/heads/master@{#434911}