tabing inconsistencies between trays in status area

Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray. This is because there is a focus traversal in web notification tray's child. Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted. The initialization sibling relation may not be ready. Add a setnextfocusibleview call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Committed: https://crrev.com/ab543ef12f3b412b9e008fce7e78f4456de82822
Cr-Original-Commit-Position: refs/heads/master@{#401765}
Cr-Commit-Position: refs/heads/master@{#401810}

[Qiang(Joe) Xu, 2016-06-21 00:13:12]

Description was changed from

==========
tabing inconsistencies between systray and web notification tray

BUG=468969
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are two bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.
==========

[Qiang(Joe) Xu, 2016-06-21 00:13:12]

warx@chromium.org changed reviewers:
+ jennyz@chromium.org

[Qiang(Joe) Xu, 2016-06-21 00:14:35]

Hi Jenny, could you help review on this? tks

[Qiang(Joe) Xu, 2016-06-21 05:05:18]

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

[commit-bot: I haz the power, 2016-06-21 05:05:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2087703002/1

[commit-bot: I haz the power, 2016-06-21 05:46:34]

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

[commit-bot: I haz the power, 2016-06-21 05:46:35]

Dry run: This issue passed the CQ dry run.

[Qiang(Joe) Xu, 2016-06-23 21:13:00]

Description was changed from

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are two bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.


(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.
==========

[Qiang(Joe) Xu, 2016-06-23 21:14:34]

Updated subject, including the case for virtual keyboard tray

[jennyz, 2016-06-23 23:15:00]

lgtm

[Qiang(Joe) Xu, 2016-06-23 23:28:18]

The CQ bit was checked by warx@chromium.org

[commit-bot: I haz the power, 2016-06-23 23:29:09]

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

[commit-bot: I haz the power, 2016-06-24 00:59:52]

Description was changed from

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.


(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.


(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.
==========

[commit-bot: I haz the power, 2016-06-24 00:59:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-24 01:02:33]

Description was changed from

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.


(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}
==========

[commit-bot: I haz the power, 2016-06-24 01:02:34]

Patchset 3 (id:??) landed as
https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}

[falken, 2016-06-24 02:03:55]

On 2016/06/24 01:02:34, commit-bot: I haz the power wrote:
> Patchset 3 (id:??) landed as
> https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
> Cr-Commit-Position: refs/heads/master@{#401765}

I think this collided with https://codereview.chromium.org/2091933002/ and broke
the build:
c:\b\build\slave\win\build\src\ash\system\web_notification\web_notification_tray.cc(510):
error C3861: 'status_area_widget': identifier not found
https://build.chromium.org/p/chromium/buildstatus?builder=Win&number=44650

[falken, 2016-06-24 02:07:05]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2090353005/ by falken@chromium.org.

The reason for reverting is: I think this collided with
https://codereview.chromium.org/2091933002/ and broke
the build:
c:\b\build\slave\win\build\src\ash\system\web_notification\web_notification_tray.cc(510):
error C3861: 'status_area_widget': identifier not found
https://build.chromium.org/p/chromium/buildstatus?builder=Win&number=44650.

[Qiang(Joe) Xu, 2016-06-24 02:31:21]

Description was changed from

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}
==========

[Qiang(Joe) Xu, 2016-06-24 02:31:22]

warx@chromium.org changed reviewers:
+ falken@chromium.org

[Qiang(Joe) Xu, 2016-06-24 02:32:45]

On 2016/06/24 02:07:05, falken wrote:
> A revert of this CL (patchset #3 id:40001) has been created in
> https://codereview.chromium.org/2090353005/ by mailto:falken@chromium.org.
> 
> The reason for reverting is: I think this collided with
> https://codereview.chromium.org/2091933002/ and broke
> the build:
>
c:\b\build\slave\win\build\src\ash\system\web_notification\web_notification_tray.cc(510):
> error C3861: 'status_area_widget': identifier not found
> https://build.chromium.org/p/chromium/buildstatus?builder=Win&number=44650.

rebase on latest and fix the conflict, falken@ or jennyz@, PTAL, thanks!

[James Cook, 2016-06-24 02:48:09]

jamescook@chromium.org changed reviewers:
+ jamescook@chromium.org

[James Cook, 2016-06-24 02:48:10]

LGTM.  You might need to re-rebase, as my patch got reverted too.

[James Cook, 2016-06-24 02:58:30]

lgtm again

[Qiang(Joe) Xu, 2016-06-24 03:01:53]

The CQ bit was checked by warx@chromium.org

[Qiang(Joe) Xu, 2016-06-24 03:01:53]

The patchset sent to the CQ was uploaded after l-g-t-m from jennyz@chromium.org
Link to the patchset: https://codereview.chromium.org/2087703002/#ps80001
(title: "land mine first")

[commit-bot: I haz the power, 2016-06-24 03:02:17]

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

[commit-bot: I haz the power, 2016-06-24 03:51:40]

Description was changed from

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}
==========

[commit-bot: I haz the power, 2016-06-24 03:51:42]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-24 03:53:22]

Description was changed from

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Cr-Commit-Position: refs/heads/master@{#401765}
==========

to

==========
Detailed reproduce steps are in comment 2 in crbug.com/468969

There are several bugs in the status area tray focus advancing.

(1) there is a invisible focus between system tray and web notification tray.
This is because there is a focus traversal in web notification tray's child.
Solution is to disable it.
When virtual keyboard is enabled, we should also disable its child view
traversal.

(2) TAB will be trapped in system tray. Web notification tray is async painted.
The initialization sibling relation may not be ready. Add a setnextfocusibleview
call will fix that.

BUG=468969
TEST=tested on device, both tabing and shift-tabing works well on status area.

There is still a bug in crbug.com/468969 comment 7. It happens in login screen
only. When logged in, it works fine.

Committed: https://crrev.com/133c792af9dfe8c82561bee7015efcc0b5dba22d
Committed: https://crrev.com/ab543ef12f3b412b9e008fce7e78f4456de82822
Cr-Original-Commit-Position: refs/heads/master@{#401765}
Cr-Commit-Position: refs/heads/master@{#401810}
==========

[commit-bot: I haz the power, 2016-06-24 03:53:23]

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

[blundell, 2016-06-24 10:33:54]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2095933002/ by blundell@chromium.org.

The reason for reverting is: Causes UaF in web_notification_tray.cc on
focus_cycler_unittest.cc. The issue appears to be unclear lifetime assumptions
of the system tray object: the status area widget maintains a raw pointer to it
(and confusingly *sometimes* owns it, i.e. when its Shutdown() method is
called), whereas focus_cycler_unittest.cc actually owns it but doesn't clear it
out from the status area widget when it deletes it on TearDown(). It looks like
this CL just tickled a pre-existing bad situation, but the best fix is not
obvious to me as a non-expert in this part of the codebase, so I have to revert.

Example failure:
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...

Relevant snippet:

==7090==ERROR: AddressSanitizer: heap-use-after-free on address 0x618000021608
at pc 0x00000a32db44 bp 0x7ffdd1268ae0 sp 0x7ffdd1268ad8
WRITE of size 8 at 0x618000021608 thread T0
    #0 0xa32db43 in views::View::SetNextFocusableView(views::View*)
ui/views/view.cc:1198:24
    #1 0x964d8ba in Run base/callback.h:397:12
    #2 0x964d8ba in base::debug::TaskAnnotator::RunTask(char const*,
base::PendingTask const&) base/debug/task_annotator.cc:51
    #3 0x95131cc in base::MessageLoop::RunTask(base::PendingTask const&)
base/message_loop/message_loop.cc:493:19
    #4 0x951401b in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask
const&) base/message_loop/message_loop.cc:502:5
    #5 0x9514eab in base::MessageLoop::DoWork()
base/message_loop/message_loop.cc:624:13
    #6 0x95191f7 in base::MessagePumpGlib::Run(base::MessagePump::Delegate*)
base/message_loop/message_pump_glib.cc:313:49
    #7 0x956bbfb in base::RunLoop::Run() base/run_loop.cc:35:10
    #8 0x127dc12 in ash::test::AshTestHelper::RunAllPendingInMessageLoop()
ash/test/ash_test_helper.cc:192:12
    #9 0x127b887 in RunAllPendingInMessageLoop ash/test/ash_test_base.cc:287:21
    #10 0x127b887 in ash::test::AshTestBase::TearDown()
ash/test/ash_test_base.cc:163.

[James Cook, 2016-06-24 15:49:05]

On 2016/06/24 10:33:54, blundell wrote:
> A revert of this CL (patchset #5 id:80001) has been created in
> https://codereview.chromium.org/2095933002/ by mailto:blundell@chromium.org.
> 
> The reason for reverting is: Causes UaF in web_notification_tray.cc on
> focus_cycler_unittest.cc. The issue appears to be unclear lifetime assumptions
> of the system tray object: the status area widget maintains a raw pointer to
it
> (and confusingly *sometimes* owns it, i.e. when its Shutdown() method is
> called), whereas focus_cycler_unittest.cc actually owns it but doesn't clear
it
> out from the status area widget when it deletes it on TearDown(). It looks
like
> this CL just tickled a pre-existing bad situation, but the best fix is not
> obvious to me as a non-expert in this part of the codebase, so I have to
revert.
> 
> Example failure:
>
https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%2...
> 
> Relevant snippet:
> 
> ==7090==ERROR: AddressSanitizer: heap-use-after-free on address 0x618000021608
> at pc 0x00000a32db44 bp 0x7ffdd1268ae0 sp 0x7ffdd1268ad8
> WRITE of size 8 at 0x618000021608 thread T0
>     #0 0xa32db43 in views::View::SetNextFocusableView(views::View*)
> ui/views/view.cc:1198:24
>     #1 0x964d8ba in Run base/callback.h:397:12
>     #2 0x964d8ba in base::debug::TaskAnnotator::RunTask(char const*,
> base::PendingTask const&) base/debug/task_annotator.cc:51
>     #3 0x95131cc in base::MessageLoop::RunTask(base::PendingTask const&)
> base/message_loop/message_loop.cc:493:19
>     #4 0x951401b in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask
> const&) base/message_loop/message_loop.cc:502:5
>     #5 0x9514eab in base::MessageLoop::DoWork()
> base/message_loop/message_loop.cc:624:13
>     #6 0x95191f7 in base::MessagePumpGlib::Run(base::MessagePump::Delegate*)
> base/message_loop/message_pump_glib.cc:313:49
>     #7 0x956bbfb in base::RunLoop::Run() base/run_loop.cc:35:10
>     #8 0x127dc12 in ash::test::AshTestHelper::RunAllPendingInMessageLoop()
> ash/test/ash_test_helper.cc:192:12
>     #9 0x127b887 in RunAllPendingInMessageLoop
ash/test/ash_test_base.cc:287:21
>     #10 0x127b887 in ash::test::AshTestBase::TearDown()
> ash/test/ash_test_base.cc:163.

Perhaps FocusCyclerTest needs to call StatusAreaWidget::Shutdown() on the one it
creates?

Failing that, perhaps it should use the real StatusAreaWidget created by
AshTestBase.

[Qiang(Joe) Xu, 2016-06-24 21:48:21]

Patchset #6 (id:100001) has been deleted

[Qiang(Joe) Xu, 2016-06-24 22:55:13]

Patchset #6 (id:120001) has been deleted