WidgetDelegateView is its own contents view.

WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do so.

BUG=none
TBR=mkwst@chromium.org, tapted@chromium.org

Committed: https://crrev.com/9b564da108e489972a86c602cffb0a786d4a5991
Cr-Commit-Position: refs/heads/master@{#423583}

[Evan Stade, 2016-10-04 20:08:12]

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

[commit-bot: I haz the power, 2016-10-04 20:08:47]

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-10-04 20:45:32]

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

[commit-bot: I haz the power, 2016-10-04 20:45:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2016-10-05 18:39:23]

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

[commit-bot: I haz the power, 2016-10-05 18:39:43]

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-10-05 19:23:26]

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

[commit-bot: I haz the power, 2016-10-05 19:23:27]

Dry run: This issue passed the CQ dry run.

[Evan Stade, 2016-10-05 19:39:43]

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

[Evan Stade, 2016-10-05 19:40:05]

Description was changed from

==========
WidgetDelegateView is its own contents view.

BUG=
==========

to

==========
WidgetDelegateView is its own contents view.

BUG=none
==========

[commit-bot: I haz the power, 2016-10-05 19:40:27]

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-10-05 19:59:40]

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

[commit-bot: I haz the power, 2016-10-05 19:59:41]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Evan Stade, 2016-10-05 23:46:15]

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

[Evan Stade, 2016-10-05 23:46:37]

Description was changed from

==========
WidgetDelegateView is its own contents view.

BUG=none
==========

to

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
==========

[Evan Stade, 2016-10-05 23:46:38]

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

[commit-bot: I haz the power, 2016-10-05 23:46:45]

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-10-06 00:05:34]

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

[commit-bot: I haz the power, 2016-10-06 00:05:35]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[sky, 2016-10-06 16:49:20]

THANK YOU! The lack of WidgetDelegateView overriding GetContentsView() has long
bothered me but I've been too lazy to fix it.

LGTM

https://codereview.chromium.org/2390953004/diff/60001/ui/views/touchui/touch_...
File ui/views/touchui/touch_selection_controller_impl.cc (right):

https://codereview.chromium.org/2390953004/diff/60001/ui/views/touchui/touch_...
ui/views/touchui/touch_selection_controller_impl.cc:323: if
(selection_bound_.type() == gfx::SelectionBound::EMPTY)
It would be nice if you had a comment as to why this is here to save me the time
of verifying it's necessary.

[Evan Stade, 2016-10-06 16:51:44]

https://codereview.chromium.org/2390953004/diff/60001/ui/views/touchui/touch_...
File ui/views/touchui/touch_selection_controller_impl.cc (right):

https://codereview.chromium.org/2390953004/diff/60001/ui/views/touchui/touch_...
ui/views/touchui/touch_selection_controller_impl.cc:323: if
(selection_bound_.type() == gfx::SelectionBound::EMPTY)
On 2016/10/06 16:49:20, sky wrote:
> It would be nice if you had a comment as to why this is here to save me the
time
> of verifying it's necessary.

Done.

[Evan Stade, 2016-10-06 16:53:11]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2016-10-06 16:53:11]

The patchset sent to the CQ was uploaded after l-g-t-m from sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2390953004/#ps100001
(title: "another minor fix")

[commit-bot: I haz the power, 2016-10-06 16:53:26]

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-10-06 17:04:03]

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

[commit-bot: I haz the power, 2016-10-06 17:04:04]

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

[Evan Stade, 2016-10-06 17:46:11]

Description was changed from

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
==========

to

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
TBR=mkwst@chromium.org, tapted@chromium.org
==========

[Evan Stade, 2016-10-06 17:46:11]

estade@chromium.org changed reviewers:
+ mkwst@chromium.org, tapted@chromium.org

[Evan Stade, 2016-10-06 17:46:26]

added TBRs for:

tapted:
  extensions/components/native_app_window/native_app_window_views.cc
  extensions/components/native_app_window/native_app_window_views.h
mkwst:
  content/shell/browser/shell_views.cc

[Evan Stade, 2016-10-06 17:46:37]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2016-10-06 17:47:09]

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-10-06 17:56:38]

Description was changed from

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
TBR=mkwst@chromium.org, tapted@chromium.org
==========

to

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
TBR=mkwst@chromium.org, tapted@chromium.org
==========

[commit-bot: I haz the power, 2016-10-06 17:56:40]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-10-06 17:59:19]

Description was changed from

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
TBR=mkwst@chromium.org, tapted@chromium.org
==========

to

==========
WidgetDelegateView is its own contents view.

Therefore, make it override GetContentsView instead of making all subclasses do
so.

BUG=none
TBR=mkwst@chromium.org, tapted@chromium.org

Committed: https://crrev.com/9b564da108e489972a86c602cffb0a786d4a5991
Cr-Commit-Position: refs/heads/master@{#423583}
==========

[commit-bot: I haz the power, 2016-10-06 17:59:21]

Patchset 6 (id:??) landed as
https://crrev.com/9b564da108e489972a86c602cffb0a786d4a5991
Cr-Commit-Position: refs/heads/master@{#423583}

[tapted, 2016-10-07 04:48:11]

native_app_window_views lgtm

https://codereview.chromium.org/2390953004/diff/100001/ui/views/widget/widget...
File ui/views/widget/widget_delegate.h (right):

https://codereview.chromium.org/2390953004/diff/100001/ui/views/widget/widget...
ui/views/widget/widget_delegate.h:214: views::View* GetContentsView() override;
(nit: views:: prefix isn't needed here, or in the .cc)