Reland performance fixes for StatusBubbleMac.

Reland performance fixes for StatusBubbleMac.

This reverts commit 7a15c8173fca5413fe61113b82185fe721ce0b98, bringing
back the following changes:

[Mac] Always sets the status bubble to 1pt wide when hidden.

When the status bubble is in state kBubbleHidden, its width is now always 1pt.
This prevents us from wasting time resizing and redrawing a hidden bubble.

[Mac] Cleans up the StatusBubbleMac code.

This CL reduces unnecessary work in StatusBubbleMac and replaces it with some
logging to track down when invariants are violated.  (The presumed invariant
here is that we set kWindowSizeDeterminedLater once, when state is set to
kBubbleHidden, and the size stays that way until the bubble transitions to a
different state.  If that invariant held, then we wouldn't need to constantly
re-set the size for a hidden bubble.  This invariant is being violated on the
GPU bots, so the logging is in place to try to find and fix the cause.)

BUG=454502
BUG=467998
TEST=No visible impact.  Status bubble should continue to show when hovering over links and resize properly when the window size is changed.
TEST=No visible impact.  Status bubble should continue to show, expand, and hide properly.

Committed: https://crrev.com/da19048a6cd7ebd52e77510d0e787e2ef458b833
Cr-Commit-Position: refs/heads/master@{#327191}

[rohitrao (ping after 24h), 2015-04-27 19:29:53]

rohitrao@chromium.org changed reviewers:
+ erikchen@chromium.org

[rohitrao (ping after 24h), 2015-04-27 19:29:54]

Not for a real review.

Delta between PS1 and PS2 shows the problem and a super-hacky fix.

In UpdateSizeAndPosition(), we were returning early if the bubble was hidden,
leaving the frame unchanged (except in some rare "gpu bot" cases).  This is
because the size should already have been (1,1), so no updates were needed.

This falls apart during a fullscreen animation transition, because even though
the size is correct, the *origin* of the bubble needs to change.  My CL was
leaving the bubble rooted for the old non-fullscreen window, which explains why
the transition animation got so wonky.

The old code was always computing a new bubble frame, which included fixing up
the origin in the cases that needed it.  My hack always computes a new origin,
and forces the size to (1,1) when the bubble is hidden.  This does a lot of
wasteful math, but it's enough to show that the bug goes away.

Ideally I'd want to add a method to force computation of a new origin and call
that from somewhere in the fullscreen code.

[rohitrao (ping after 24h), 2015-04-27 22:26:09]

https://codereview.chromium.org/1103323002/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):

https://codereview.chromium.org/1103323002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/status_bubble_mac.mm:788: frame.origin = [parent_
convertBaseToScreen:([parent_ frame].origin)];
I realized that the actual origin doesn't matter quite so much, as long as it's
within the parent window somewhere.  The SetFrameAvoidingMouse() code has logic
to handle the download shelf -- we don't really care about that, because the
bubble is hidden anyways.  Simply using the parent's origin should be just fine
(from the perspective of AppKit, it doesn't care if the download shelf is
present or absent anyways).

[erikchen, 2015-04-27 22:30:59]

On 2015/04/27 22:26:09, rohitrao wrote:
>
https://codereview.chromium.org/1103323002/diff/40001/chrome/browser/ui/cocoa...
> File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):
> 
>
https://codereview.chromium.org/1103323002/diff/40001/chrome/browser/ui/cocoa...
> chrome/browser/ui/cocoa/status_bubble_mac.mm:788: frame.origin = [parent_
> convertBaseToScreen:([parent_ frame].origin)];
> I realized that the actual origin doesn't matter quite so much, as long as
it's
> within the parent window somewhere.  The SetFrameAvoidingMouse() code has
logic
> to handle the download shelf -- we don't really care about that, because the
> bubble is hidden anyways.  Simply using the parent's origin should be just
fine
> (from the perspective of AppKit, it doesn't care if the download shelf is
> present or absent anyways).

Oh interesting. Maybe this is related to how AppKit takes screenshots of a
window (e.g. CGWindowListCreateImage). It might have assumptions that the
screenshot that comes out of CGWindowListCreateImage is the same size as the
window itself. Just taking a wild guess here, as I don't know how
CGWindowListCreateImage interacts with child windows.

[erikchen, 2015-04-27 22:33:26]

https://codereview.chromium.org/1103323002/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):

https://codereview.chromium.org/1103323002/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/status_bubble_mac.mm:789: [window_ setFrame:frame
display:YES];
display:YES forces a redraw, even if the origin hasn't changed. Maybe we should
only perform a redraw if the frame has actually changed?

[rohitrao (ping after 24h), 2015-04-27 23:29:48]

https://codereview.chromium.org/1103323002/diff/60001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):

https://codereview.chromium.org/1103323002/diff/60001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/status_bubble_mac.mm:789: [window_ setFrame:frame
display:NO];
Updated to not force a display.  Setting this to NO does not seem to cause
animation glitches.

[rohitrao (ping after 24h), 2015-04-27 23:30:00]

The CQ bit was checked by rohitrao@chromium.org to run a CQ dry run

[rohitrao (ping after 24h), 2015-04-27 23:30:25]

Updated to fix a bug and stop forcing a display.

[commit-bot: I haz the power, 2015-04-27 23:30:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1103323002/60001

[erikchen, 2015-04-27 23:33:11]

lgtm

[commit-bot: I haz the power, 2015-04-28 00:10:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-28 00:10:24]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2015-04-28 00:26:48]

The CQ bit was checked by rohitrao@chromium.org

[commit-bot: I haz the power, 2015-04-28 00:27:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1103323002/60001

[commit-bot: I haz the power, 2015-04-28 00:28:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-28 00:29:47]

Patchset 4 (id:??) landed as
https://crrev.com/da19048a6cd7ebd52e77510d0e787e2ef458b833
Cr-Commit-Position: refs/heads/master@{#327191}