Reland Fix MenuController Heap-use-after-free

Reland Fix MenuController Heap-use-after-free

The original patch fixed a heap-use-after-free, but exposed a memory leak.

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.

However it is possible that during drag-and-drop that two MenuControllers are
being used. The drag-and-drop can be caused by once, which is replaced by a
different active instance. The active instance can be deleted first. Upon the
completion of drag-and-drop the remaining MenuController must notify its
delegate, which can outlive it. So that associated cleanup can be done.

This change updates the ExitAsyncRun to cache heap variables needed by the
longer life delegate. So that it can still be called for cleanup if the
MenuController has been deleted.

This relands https://codereview.chromium.org/2636293002/
This reverts commit cd4f55690437f7c40bfa710586781ec316cc889e.

TEST=MenuControllerTest.DestroyedDuringViewsRelease,
BookmarkBarViewTest7.DNDToDifferentMenu,
BookmarkBarViewTest8.DNDBackToOriginatingMenu
BUG=682109, 681462
Review-Url: https://codereview.chromium.org/2641983003
Cr-Commit-Position: refs/heads/master@{#444580}
Committed: https://chromium.googlesource.com/chromium/src/+/4a3f02d18631fc413399a5e058b89f34628826d9

[jonross, 2017-01-18 22:43:14]

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

[jonross, 2017-01-18 22:43:15]

Hey sky@,

This is a follow up to the revert patch from last night. First upload is the
revert. Second is the fix.

Could you take a look?

Thanks,
Jon

[sky, 2017-01-18 23:36:10]

LGTM

[jonross, 2017-01-18 23:55:08]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2017-01-18 23:55:46]

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-19 01:06:16]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484783708088090,
"parent_rev": "b9ee881336aafb25b77a02b0620605696e2200bc", "commit_rev":
"4a3f02d18631fc413399a5e058b89f34628826d9"}

[commit-bot: I haz the power, 2017-01-19 01:07:31]

Description was changed from

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

The original patch fixed a heap-use-after-free, but exposed a memory leak.

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.

However it is possible that during drag-and-drop that two MenuControllers are
being used. The drag-and-drop can be caused by once, which is replaced by a
different active instance. The active instance can be deleted first. Upon the
completion of drag-and-drop the remaining MenuController must notify its
delegate, which can outlive it. So that associated cleanup can be done.

This change updates the ExitAsyncRun to cache heap variables needed by the
longer life delegate. So that it can still be called for cleanup if the
MenuController has been deleted.

This relands https://codereview.chromium.org/2636293002/
This reverts commit cd4f55690437f7c40bfa710586781ec316cc889e.

TEST=MenuControllerTest.DestroyedDuringViewsRelease,
BookmarkBarViewTest7.DNDToDifferentMenu,
BookmarkBarViewTest8.DNDBackToOriginatingMenu
BUG=682109, 681462
==========

to

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

The original patch fixed a heap-use-after-free, but exposed a memory leak.

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.

However it is possible that during drag-and-drop that two MenuControllers are
being used. The drag-and-drop can be caused by once, which is replaced by a
different active instance. The active instance can be deleted first. Upon the
completion of drag-and-drop the remaining MenuController must notify its
delegate, which can outlive it. So that associated cleanup can be done.

This change updates the ExitAsyncRun to cache heap variables needed by the
longer life delegate. So that it can still be called for cleanup if the
MenuController has been deleted.

This relands https://codereview.chromium.org/2636293002/
This reverts commit cd4f55690437f7c40bfa710586781ec316cc889e.

TEST=MenuControllerTest.DestroyedDuringViewsRelease,
BookmarkBarViewTest7.DNDToDifferentMenu,
BookmarkBarViewTest8.DNDBackToOriginatingMenu
BUG=682109, 681462

Review-Url: https://codereview.chromium.org/2641983003
Cr-Commit-Position: refs/heads/master@{#444580}
Committed:
https://chromium.googlesource.com/chromium/src/+/4a3f02d18631fc413399a5e058b8...
==========

[commit-bot: I haz the power, 2017-01-19 01:07:32]

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