Fixes a crash when InkDropDelegate gets destroyed after its InkDropHost

chrome/browser/ui/views/toolbar/toolbar_action_view.cc

Index: chrome/browser/ui/views/toolbar/toolbar_action_view.cc
diff --git a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
index 524fb9525cedfeabefa855b52dad6f68772f4f7d..b23c1028307fc9899f1e5f6f83c2a075a6ac0265 100644
--- a/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
+++ b/chrome/browser/ui/views/toolbar/toolbar_action_view.cc
@@ -109,6 +109,7 @@ ToolbarActionView::~ToolbarActionView() {
   if (context_menu_owner == this)
     context_menu_owner = nullptr;
   view_controller_->SetDelegate(nullptr);
+  SetInkDropDelegate(scoped_ptr<views::InkDropDelegate>());

[Devlin, 2015/12/08 16:56:37]

Would it be better to move this to the CustomButton dtor so that subclasses
don't have to worry about it?

[varkha, 2015/12/08 17:09:15]

Would be nice but this is exactly what this CL is trying to fix. The concrete
implementations of InkDropDelegate may need to call into the concrete View (such
as this ToolbarActionView class) in their destructors so ToolbarActionView still
needs to be around which is not the case by the time the base
CustomButton::~CustomButton is getting called.

[Devlin, 2015/12/08 17:33:39]

Ah, gotcha.  Well, that's disappointing.

So, let's experiment with some C++ dirty details!

We get a pure virtual when ~InkDropDelegate() is called in ~CustomButton.  But
if we were to have the InkDropDelegate owned by the ToolbarActionView, then it's
called between ~ToolbarActionView and ~CustomButton - but I don't know when the
vptr is moved.  Can you quickly see what happens if we have the InkDropDelegate
owned by ToolbarActionView, and CustomButton::SetInkDropDelegate take a
InkDropDelegate*?  (Assuming the unittest you add is right, you'll get a quick
answer :))

[varkha, 2015/12/08 19:00:50]

Yeah, I've tried it before and initially rejected it as it required a little bit
more of handholding through the destruction sequence. Please see if that is what
you've meant and if you in fact like that better. I've restored to that version
in the next patch set.

[Devlin, 2015/12/08 19:44:06]

My hope was that if ToolbarActionView owns the InkDropDelegate, we don't need to
explicitly reset() it in the destructor, since it will be destroyed *before*
~CustomButton.

On 2015/12/08 19:09:26, varkha wrote:
Right - but if we have ToolbarActionView own it, it should be cleaned up prior
to the ~CustomButton.

 }
 
 void ToolbarActionView::GetAccessibleState(ui::AXViewState* state) {