Reland 179231

Reland 179231

> views: Fix a Widget destruction crash.
>
> For WIDGET_OWNS_NATIVE_WIDGET widget, we destroy root view before we delete
> |native_widget_|. If a WidgetDelegateView is in root view's hierarchy, the
> native widget destruction would crash because |widget_delegate_| is accessed
> in OnNativeWidgetDestroying but it is deleted with root view already.
> 
> This CL marks WidgetDelegateView as owned by client so that it does not go
> away with the root view hierarchy and be deleted on DeleteDelegate.
> 
> BUG=164791
> TEST=Covered with a new unit test.
> 
> R=ben@chromium.org

BUG=164791, 172842
TEST=No memory leak as in 172842.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=179694

[xiyuan, 2013-01-23 23:46:28]

Let me know what you think. Thanks.

https://codereview.chromium.org/11953066/diff/1/ash/launcher/launcher.cc
File ash/launcher/launcher.cc (right):

https://codereview.chromium.org/11953066/diff/1/ash/launcher/launcher.cc#newc...
ash/launcher/launcher.cc:99: class Launcher::DimmerView : public views::View,
DimmerView is passed as a delegate when creating its widget. So it does not need
to be a WidgetDelegateView.

[Ben Goodger (Google), 2013-01-28 17:31:14]

lgtm

https://codereview.chromium.org/11953066/diff/1010/ui/views/widget/widget_del...
File ui/views/widget/widget_delegate.h (right):

https://codereview.chromium.org/11953066/diff/1010/ui/views/widget/widget_del...
ui/views/widget/widget_delegate.h:167: // implementation is-a View.
you should note the ownership here.

[xiyuan, 2013-01-28 18:50:09]

https://codereview.chromium.org/11953066/diff/1010/ui/views/widget/widget_del...
File ui/views/widget/widget_delegate.h (right):

https://codereview.chromium.org/11953066/diff/1010/ui/views/widget/widget_del...
ui/views/widget/widget_delegate.h:167: // implementation is-a View.
On 2013/01/28 17:31:14, Ben Goodger (Google) wrote:
> you should note the ownership here.

Done.

[commit-bot: I haz the power, 2013-01-28 19:20:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/11953066/16001

[commit-bot: I haz the power, 2013-01-28 20:34:12]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-01-28 22:35:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/11953066/10004

[commit-bot: I haz the power, 2013-01-29 00:56:11]

Change committed as 179231

[xiyuan, 2013-01-30 18:14:04]

valgrind bot is broken. Run the tests locally and the leak seems fixed.

[commit-bot: I haz the power, 2013-01-30 18:16:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/11953066/9004

[commit-bot: I haz the power, 2013-01-30 20:57:05]

Change committed as 179694