Check for null |tabstrip_| in BrowserView::IsTabStripVisible()

chrome/browser/ui/views/frame/browser_view.cc

 // Copyright 2012 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.
 
 #include "chrome/browser/ui/views/frame/browser_view.h"
 
 #include <stdint.h>
 #include <algorithm>
 #include <utility>
 
@@ skipping 559 matching lines @@
 }
 
 gfx::Rect BrowserView::GetFindBarBoundingBox() const {
   return GetBrowserViewLayout()->GetFindBarBoundingBox();
 }
 
 int BrowserView::GetTabStripHeight() const {
   // We want to return tabstrip_->height(), but we might be called in the midst
   // of layout, when that hasn't yet been updated to reflect the current state.
   // So return what the tabstrip height _ought_ to be right now.
-  return tabstrip_ && IsTabStripVisible() ?
-      tabstrip_->GetPreferredSize().height() : 0;
+  return IsTabStripVisible() ? tabstrip_->GetPreferredSize().height() : 0;
 }
 
 gfx::Point BrowserView::OffsetPointForToolbarBackgroundImage(
     const gfx::Point& point) const {
   // The background image starts tiling horizontally at the window left edge and
   // vertically at the top edge of the horizontal tab strip (or where it would
   // be).  We expect our parent's origin to be the window origin.
   gfx::Point window_point(point + GetMirroredPosition().OffsetFromOrigin());
   window_point.Offset(frame_->GetThemeBackgroundXInset(),
                       -frame_->GetTopInset(false));
   return window_point;
 }
 
 bool BrowserView::IsTabStripVisible() const {
   if (immersive_mode_controller_->ShouldHideTopViews() &&
-      immersive_mode_controller_->ShouldHideTabIndicators())
+      immersive_mode_controller_->ShouldHideTabIndicators()) {
     return false;
-  return browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP);
+  }
+  return tabstrip_ &&
+         browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP);

[Peter Kasting, 2016/03/11 22:19:50]

Nit: I might reverse the order of these two conditions, since the
SupportsWindowFeature() call is more of the "do we conceptually have a visible
tabstrip" and the |tabstrip_| check is just a "and has it been created yet"
check.

We could maybe even add comments:

  // Does this window normally display a tabstrip?
  if (!browser_->SupportsWindowFeature(Browser::FEATURE_TABSTRIP))
    return false;

  // And has that tabstrip been created yet (by InitViews())?  Note that we
  // can't just check this alone, as the tabstrip is created unconditionally
  // even for windows that won't display it.
  return tabstrip_ != nullptr;

[tdanderson, 2016/03/14 20:38:33]

Done.

 }
 
 bool BrowserView::IsOffTheRecord() const {
   return browser_->profile()->IsOffTheRecord();
 }
 
 bool BrowserView::IsGuestSession() const {
   return browser_->profile()->IsGuestSession();
 }
 
@@ skipping 2089 matching lines @@
 }
 
 extensions::ActiveTabPermissionGranter*
 BrowserView::GetActiveTabPermissionGranter() {
   content::WebContents* web_contents = GetActiveWebContents();
   if (!web_contents)
     return nullptr;
   return extensions::TabHelper::FromWebContents(web_contents)
       ->active_tab_permission_granter();
 }