Make sure bubbles in Views default to close before their RenderFrameHosts.

components/bubble/bubble_close_reason.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_BUBBLE_BUBBLE_CLOSE_REASON_H_
 #define COMPONENTS_BUBBLE_BUBBLE_CLOSE_REASON_H_
 
 // List of reasons why a bubble might close. These correspond to various events
 // from the UI. Not all platforms will receive all events.
 enum BubbleCloseReason {
   // The bubble WILL be closed regardless of return value for |ShouldClose|.
   // Ex: The bubble's parent window is being destroyed.
   BUBBLE_CLOSE_FORCED,

[Peter Kasting, 2016/01/09 01:19:35]

Can't we just use this?  We only need the new value to track this in a
histogram, but it's not clear we need that to be distinct from this value in the
histogram.

[Jeffrey Yasskin, 2016/01/09 02:22:34]

If a bubble needs to avoid closing when a frame is closed, (like the extensions
bubble avoids closing for navigation), it'll need to use the new value. I guess
you're saying that we shouldn't think ahead, and instead let that bubble add the
value when it needs it? Which of these other values would you merge?

The histogram may also be useful in telling us whether the new value was
actually needed.

[Peter Kasting, 2016/01/09 02:26:18]

Ah.
I wouldn't add the complexity if we don't know we need it.
I don't think I understand the question?  Are you asking if there are other
values here that should be cleaned up at the same time?
That would be a good reason, if you have firm plans to monitor this and then
remove the new code you're adding if it turns out to be unnecessary.

[hcarmona, 2016/01/11 19:07:22]

If we have a reason to close a bubble, it should be added here in order
to avoid having weird logic later for an overloaded close reason.
Close reasons should be explicit IMHO. This leads to simpler bubble
logic and more concise stats.

[Peter Kasting, 2016/01/11 21:42:19]

My argument is really that "force close" _is_ the reason, given the comments on
what this is supposed to mean.

If you don't think that's good, then we shouldn't have a value like "force
close" at all.  All existing uses of this should be replaced with distinct
individual values.

I don't think, in this specific case, that that results in any simpler bubble
close logic (it's more complicated because we have to check more conditions;
there's no "weird logic later") or more concise stats (again, they're actually
less concise due to having more buckets).  So I don't think that change
accomplishes your stated aims; I think what I'm arguing for is precisely what
you're claiming to want.  Am I confused?

 
   // Bubble was closed without any user interaction.
   BUBBLE_CLOSE_FOCUS_LOST,
 
   // User did not interact with the bubble, but changed tab.
   BUBBLE_CLOSE_TABSWITCHED,
 
   // User did not interact with the bubble, but detached the tab.
   BUBBLE_CLOSE_TABDETACHED,
 
   // User dismissed the bubble. (ESC, close, etc.)
   BUBBLE_CLOSE_USER_DISMISSED,
 
   // There has been a navigation event. (Link, URL typed, refresh, etc.)
   BUBBLE_CLOSE_NAVIGATED,
 
   // The parent window has entered or exited fullscreen mode. Will also be
   // called for immersive fullscreen.
   BUBBLE_CLOSE_FULLSCREEN_TOGGLED,
 
   // The user selected an affirmative response in the bubble.
   BUBBLE_CLOSE_ACCEPTED,
 
   // The user selected a negative response in the bubble.
   BUBBLE_CLOSE_CANCELED,
+
+  // A frame was closed, so references in the bubble may be invalid.

[hcarmona, 2016/01/11 19:07:22]

Let's be more specific here about what references are invalid.

+  BUBBLE_CLOSE_FRAME_DESTROYED,
 };
 
 #endif  // COMPONENTS_BUBBLE_BUBBLE_CLOSE_REASON_H_