Adjust initial activation/focus Task Manager/Views TableView.

Adjust initial activation/focus Task Manager/Views TableView.

In the task manager the table no longer has initial focus. When you open
the task manager, you'll need to press tab to focus the table.

When a TableView is focused it will activate the first row if no row is already active.

BUG=702489
Review-Url: https://codereview.chromium.org/2769243002
Cr-Commit-Position: refs/heads/master@{#459825}
Committed: https://chromium.googlesource.com/chromium/src/+/2fc3ba8587526eb5e5f7455ca463b0c166b1ee95

[Evan Stade, 2017-03-23 16:53:49]

Description was changed from

==========
Adjust initial activation/focus Task Manager/Views TableView.

In the task manager the table no longer has initial focus. When you open
the task manager, you'll need to press tab to focus the table.

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

to

==========
Adjust initial activation/focus Task Manager/Views TableView.

In the task manager the table no longer has initial focus. When you open
the task manager, you'll need to press tab to focus the table.

When a TableView is focused it will activate the first row if no row is already
active.

BUG=702489
==========

[Evan Stade, 2017-03-23 16:53:50]

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

[sky, 2017-03-23 17:50:17]

https://codereview.chromium.org/2769243002/diff/1/chrome/browser/ui/views/tas...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/2769243002/diff/1/chrome/browser/ui/views/tas...
chrome/browser/ui/views/task_manager_view.cc:165: return nullptr;
?

https://codereview.chromium.org/2769243002/diff/1/ui/views/controls/table/tab...
File ui/views/controls/table/table_view.cc (right):

https://codereview.chromium.org/2769243002/diff/1/ui/views/controls/table/tab...
ui/views/controls/table/table_view.cc:630: if (selection_model_.active() == -1)
&& RowCount() > 0 ? Maybe add test coverage for the empty model case?

[Evan Stade, 2017-03-23 22:56:26]

https://codereview.chromium.org/2769243002/diff/1/chrome/browser/ui/views/tas...
File chrome/browser/ui/views/task_manager_view.cc (right):

https://codereview.chromium.org/2769243002/diff/1/chrome/browser/ui/views/tas...
chrome/browser/ui/views/task_manager_view.cc:165: return nullptr;
On 2017/03/23 17:50:17, sky wrote:
> ?

oops, ps2 upload got stuck in the tubes.

https://codereview.chromium.org/2769243002/diff/1/ui/views/controls/table/tab...
File ui/views/controls/table/table_view.cc (right):

https://codereview.chromium.org/2769243002/diff/1/ui/views/controls/table/tab...
ui/views/controls/table/table_view.cc:630: if (selection_model_.active() == -1)
On 2017/03/23 17:50:17, sky wrote:
> && RowCount() > 0 ? Maybe add test coverage for the empty model case?

I didn't add that check because I was copying AdvanceSelection which doesn't
make the check. But it seems there are other places like OnKeyPressed that do
check, so added.

I had to move some DCHECKs in ModelToView to make the test actually catch this
problem (the selection model doesn't or care about what rows actually exist, so
it's happy to select a non existent row).

[Evan Stade, 2017-03-23 22:56:41]

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

[commit-bot: I haz the power, 2017-03-23 22:57:37]

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-03-24 00:01:13]

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

[commit-bot: I haz the power, 2017-03-24 00:01:14]

Dry run: This issue passed the CQ dry run.

[sky, 2017-03-24 02:23:51]

https://codereview.chromium.org/2769243002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/task_manager_view.h (left):

https://codereview.chromium.org/2769243002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/task_manager_view.h:57: views::View*
GetInitiallyFocusedView() override;
How come we no longer need this override? If you're relying on picking it up by
default, might that change? Seems better to make it explicit.

[Evan Stade, 2017-03-27 15:17:09]

https://codereview.chromium.org/2769243002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/task_manager_view.h (left):

https://codereview.chromium.org/2769243002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/task_manager_view.h:57: views::View*
GetInitiallyFocusedView() override;
On 2017/03/24 02:23:50, sky wrote:
> How come we no longer need this override? If you're relying on picking it up
by
> default, might that change? Seems better to make it explicit.

Yes. It relies on the default being null. I don't know how that would change
because I can't think of what it could change to. I suspect there are many
WidgetDelegates that rely on the default being null, but if we did for some
reason change that we'd probably want that new default to apply to them all,
including this one.

[sky, 2017-03-27 16:11:53]

Ok, LGTM

[Evan Stade, 2017-03-27 16:46:16]

The CQ bit was checked by estade@chromium.org

[commit-bot: I haz the power, 2017-03-27 16:46:50]

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-03-27 18:02:04]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490633176914960,
"parent_rev": "b9f7581b93ae8096afed29d415791803caf2e2e3", "commit_rev":
"2fc3ba8587526eb5e5f7455ca463b0c166b1ee95"}

[commit-bot: I haz the power, 2017-03-27 18:02:55]

Description was changed from

==========
Adjust initial activation/focus Task Manager/Views TableView.

In the task manager the table no longer has initial focus. When you open
the task manager, you'll need to press tab to focus the table.

When a TableView is focused it will activate the first row if no row is already
active.

BUG=702489
==========

to

==========
Adjust initial activation/focus Task Manager/Views TableView.

In the task manager the table no longer has initial focus. When you open
the task manager, you'll need to press tab to focus the table.

When a TableView is focused it will activate the first row if no row is already
active.

BUG=702489

Review-Url: https://codereview.chromium.org/2769243002
Cr-Commit-Position: refs/heads/master@{#459825}
Committed:
https://chromium.googlesource.com/chromium/src/+/2fc3ba8587526eb5e5f7455ca463...
==========

[commit-bot: I haz the power, 2017-03-27 18:02:56]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/2fc3ba8587526eb5e5f7455ca463...