Refactor OpaqueBrowserFrameView; reduce uses of kUseSystemTitleBar.

Refactor OpaqueBrowserFrameView; reduce uses of kUseSystemTitleBar.

This brings the number of places in the codebase that use the
kUseSystemTitleBar switch variable down to two (down from five).

This will reduce the number of places that need to be changed to replace
this flag with a preference.

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

[Matt Giuca, 2014-01-24 05:55:10]

I'm preparing to switch this flag over to a preference. It's much easier do do
this in as few places as possible (since preferences, unlike flags, do not have
global variables).

This change should not affect any behaviour.

https://codereview.chromium.org/146343002/diff/70001/chrome/browser/ui/views/...
File chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc (right):

https://codereview.chromium.org/146343002/diff/70001/chrome/browser/ui/views/...
chrome/browser/ui/views/frame/opaque_browser_frame_view_layout.cc:274: return
!delegate_->ShouldShowCaptionButtons() || delegate_->IsMaximized();
Detailed logic as to why this does not change any behaviour:

On Linux, this is equivalent to:
  kUseSystemTitleBar || (UnityIsRunning && IsMaximized) || IsMaximized
= kUseSystemTitleBar || IsMaximized

On Windows 8, this is equivalent to:
  IsSingleWindowMetroMode || IsMaximized
= IsMaximized

On other platforms, this is equivalent to:
  IsMaximized

So this refactor gives the same logic as before (but with the bonus of
offloading the decision about the kUseSystemTitleBar switch to
OpaqueBrowserFrameView).

I believe this has good future-proof semantics. The rationale for NOT condensing
the title bar is because you want extra room to display the caption buttons, so
I believe the new logic captures the intent of condensing the title bar as well
as the existing behaviour.

[James Cook, 2014-01-24 18:16:26]

LGTM

[Matt Giuca, 2014-01-30 08:47:56]

I had to update the tests. The expected output has changed because the tests
were previously nonsensical. Two tests broke (WithoutCaptionButtons and
WindowWithAvatarWithoutCaptionButtonsOnLeft) because they were testing a
non-maximized window with non-system title bar with no caption buttons (which
can never happen), and expecting a non-condensed tab strip. The behaviour has
changed so that the tab strip is now considered condensed in this situation, so
the expected dimensions have changed. This is actually a more useful set of
tests because they simulate the system title bar style, which could previously
not be faked in tests. The behaviour of the real browser should not have changed
even though the tests did.

Since this is post-LGTM, I'll wait until the end of the day (PST) in case you
think there is something wrong, but if not I'll commit it assuming the tests
pass.

[James Cook, 2014-01-30 18:10:41]

SLGTM, thanks for the detailed explanation

[commit-bot: I haz the power, 2014-01-30 23:04:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mgiuca@chromium.org/146343002/160001

[commit-bot: I haz the power, 2014-01-31 01:13:13]

Change committed as 248097

[commit-bot: I haz the power, 2014-01-31 01:13:17]

CQ bit was unchecked on CL. Ignoring.