cros: Fix clusterfuzz crash when spawning app list very early in startup

cros: Fix clusterfuzz crash when spawning app list very early in startup

The app list relies on the shelf being created for positioning. If a test
injects a search-key-press event as the very first event processed by the
message loop then the app list code that relies on the shelf may crash.

It's theoretically possible a user could hit the search key after login but
before the window manager has created the shelf. Don't rely on the existance
of the shelf app list button to init the app list.

BUG=676843
TEST=none

Review-Url: https://codereview.chromium.org/2615743002
Cr-Commit-Position: refs/heads/master@{#441673}
Committed: https://chromium.googlesource.com/chromium/src/+/64a56f0578be10ab62cdb66cbbbf62d5f306b489

[James Cook, 2017-01-04 23:03:12]

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

[commit-bot: I haz the power, 2017-01-04 23:03:49]

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

[James Cook, 2017-01-04 23:08:01]

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

[James Cook, 2017-01-04 23:08:02]

msw, please take a look. This isn't a great fix, but I doubt this crash would
happen in practice and there are no reports in go/crash.

I could also move the check to WmShell::ToggleAppList, but that would require
folding WmShell::ShowAppList and ToggleAppList together, and would also conflict
with your pending CL.

[James Cook, 2017-01-04 23:18:35]

On 2017/01/04 23:08:02, James Cook wrote:
> msw, please take a look. This isn't a great fix, but I doubt this crash would
> happen in practice and there are no reports in go/crash.
> 
> I could also move the check to WmShell::ToggleAppList, but that would require
> folding WmShell::ShowAppList and ToggleAppList together, and would also
conflict
> with your pending CL.

(Aside: I think this problem is also a sign we should always create the
shelfview, even at the login screen, and just hide it.)

[commit-bot: I haz the power, 2017-01-04 23:51:18]

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

[commit-bot: I haz the power, 2017-01-04 23:51:19]

Dry run: This issue passed the CQ dry run.

[msw, 2017-01-05 00:41:34]

lgtm as a crash workaround, but yeah, this isn't the optimal fix.

https://codereview.chromium.org/2615743002/diff/1/ash/common/accelerators/acc...
File ash/common/accelerators/accelerator_controller.cc (right):

https://codereview.chromium.org/2615743002/diff/1/ash/common/accelerators/acc...
ash/common/accelerators/accelerator_controller.cc:273: // app list depends on
the shelf for positioning. Users never open the app
The app list doesn't actually depend on the shelf for positioning (at least not
in practice anymore, afaik...). Perhaps we can remove any vestiges of that
dependency to avoid this crash?

[James Cook, 2017-01-05 03:42:09]

Description was changed from

==========
cros: Fix clusterfuzz crash when spawning app list very early in startup

The app list relies on the shelf being created for positioning. If a test
injects a search-key-press event as the very first event processed by the
message loop then the app list code that relies on the shelf may crash.

It's theoretically possible a user could hit the search key after login but
before the window manager has created the shelf. Just block showing the app
list in that rare case.

BUG=676843
TEST=none
==========

to

==========
cros: Fix clusterfuzz crash when spawning app list very early in startup

The app list relies on the shelf being created for positioning. If a test
injects a search-key-press event as the very first event processed by the
message loop then the app list code that relies on the shelf may crash.

It's theoretically possible a user could hit the search key after login but
before the window manager has created the shelf. Don't rely on the existance
of the shelf app list button to init the app list.

BUG=676843
TEST=none
==========

[James Cook, 2017-01-05 03:47:52]

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

[commit-bot: I haz the power, 2017-01-05 03:48:16]

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

[James Cook, 2017-01-05 03:50:29]

msw, please take a look at this alternate fix.

AppListPresenterDelegate continues to have shelf dependencies but I've
eliminated the dependency on AppListButton during construction, which seems to
fix the crash.

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

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

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

Dry run: This issue passed the CQ dry run.

[msw, 2017-01-05 15:43:18]

lgtm

[James Cook, 2017-01-05 16:32:53]

The CQ bit was checked by jamescook@chromium.org

[commit-bot: I haz the power, 2017-01-05 16:33:10]

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 16:39:31]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483633973510290,
"parent_rev": "c0d82cf0179b873629fb2cbf2c253e9edc591920", "commit_rev":
"64a56f0578be10ab62cdb66cbbbf62d5f306b489"}

[commit-bot: I haz the power, 2017-01-05 16:40:08]

Description was changed from

==========
cros: Fix clusterfuzz crash when spawning app list very early in startup

The app list relies on the shelf being created for positioning. If a test
injects a search-key-press event as the very first event processed by the
message loop then the app list code that relies on the shelf may crash.

It's theoretically possible a user could hit the search key after login but
before the window manager has created the shelf. Don't rely on the existance
of the shelf app list button to init the app list.

BUG=676843
TEST=none
==========

to

==========
cros: Fix clusterfuzz crash when spawning app list very early in startup

The app list relies on the shelf being created for positioning. If a test
injects a search-key-press event as the very first event processed by the
message loop then the app list code that relies on the shelf may crash.

It's theoretically possible a user could hit the search key after login but
before the window manager has created the shelf. Don't rely on the existance
of the shelf app list button to init the app list.

BUG=676843
TEST=none

Review-Url: https://codereview.chromium.org/2615743002
Cr-Commit-Position: refs/heads/master@{#441673}
Committed:
https://chromium.googlesource.com/chromium/src/+/64a56f0578be10ab62cdb66cbbbf...
==========

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

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/64a56f0578be10ab62cdb66cbbbf...