Fix MenuController Heap-use-after-free

Fix MenuController Heap-use-after-free

MenuController applies a ref to ViewsDelegate, in order to prevent Chrome from
shutting down while a menu is open. This ref is released as the menu is closing.

However it is possible for the release of the ref to lead to Chrome shutting
down immediately. During this MenuController is deleted. However it was possible
that MenuController would access the heap as the stack collapsed.

This change updates the menu closing process to detect the deletion and to
shutdown cleanly.

TEST=MenuControllerTest.DestroyedDuringViewsRelease
BUG=681462
Review-Url: https://codereview.chromium.org/2636293002
Cr-Commit-Position: refs/heads/master@{#444203}
Committed: https://chromium.googlesource.com/chromium/src/+/faaee985121bc612a5a79b03215a490f7f65d0eb

[jonross, 2017-01-17 22:40:18]

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

[jonross, 2017-01-17 22:40:19]

Hey sky@,

Could you review this change?

Thanks,
Jon

[sky, 2017-01-17 23:24:17]

Tricky. LGTM

https://codereview.chromium.org/2636293002/diff/1/ui/views/controls/menu/menu...
File ui/views/controls/menu/menu_controller.cc (right):

https://codereview.chromium.org/2636293002/diff/1/ui/views/controls/menu/menu...
ui/views/controls/menu/menu_controller.cc:2595: // Releaseing the lock can
result in Chrome shutting down, deleting this.
Releasing.

[jonross, 2017-01-17 23:29:28]

https://codereview.chromium.org/2636293002/diff/1/ui/views/controls/menu/menu...
File ui/views/controls/menu/menu_controller.cc (right):

https://codereview.chromium.org/2636293002/diff/1/ui/views/controls/menu/menu...
ui/views/controls/menu/menu_controller.cc:2595: // Releaseing the lock can
result in Chrome shutting down, deleting this.
On 2017/01/17 23:24:17, sky wrote:
> Releasing.

Done.

[jonross, 2017-01-17 23:29:39]

The CQ bit was checked by jonross@chromium.org

[jonross, 2017-01-17 23:29:39]

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/2636293002/#ps20001
(title: "Typo in comments")

[commit-bot: I haz the power, 2017-01-17 23:30:21]

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-18 00:30:41]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484695779347820,
"parent_rev": "de6e057645ab4d0085bcd65f69463b6e6b0d0c04", "commit_rev":
"faaee985121bc612a5a79b03215a490f7f65d0eb"}

[commit-bot: I haz the power, 2017-01-18 00:31:11]

Description was changed from

==========
Fix MenuController Heap-use-after-free

MenuController applies a ref to ViewsDelegate, in order to prevent Chrome from
shutting down while a menu is open. This ref is released as the menu is closing.

However it is possible for the release of the ref to lead to Chrome shutting
down immediately. During this MenuController is deleted. However it was possible
that MenuController would access the heap as the stack collapsed.

This change updates the menu closing process to detect the deletion and to
shutdown cleanly.

TEST=MenuControllerTest.DestroyedDuringViewsRelease
BUG=681462
==========

to

==========
Fix MenuController Heap-use-after-free

MenuController applies a ref to ViewsDelegate, in order to prevent Chrome from
shutting down while a menu is open. This ref is released as the menu is closing.

However it is possible for the release of the ref to lead to Chrome shutting
down immediately. During this MenuController is deleted. However it was possible
that MenuController would access the heap as the stack collapsed.

This change updates the menu closing process to detect the deletion and to
shutdown cleanly.

TEST=MenuControllerTest.DestroyedDuringViewsRelease
BUG=681462

Review-Url: https://codereview.chromium.org/2636293002
Cr-Commit-Position: refs/heads/master@{#444203}
Committed:
https://chromium.googlesource.com/chromium/src/+/faaee985121bc612a5a79b03215a...
==========

[commit-bot: I haz the power, 2017-01-18 00:31:12]

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

[meade_UTC10, 2017-01-18 05:18:15]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2638293002/ by meade@chromium.org.

The reason for reverting is: Caused a memory leak in
BookmarkBarViewTest7.DNDToDifferentMenu and
BookmarkBarViewTest8.DNDBackToOriginatingMenu

See crbug.com/682109

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