views: robustify BrowserView::GetBrowserViewForBrowser

This method is not safe to use in test contexts sometimes: it assumes that
browser->window() is a BrowserView in Views builds, when in fact it is sometimes
a TestBrowserWindow instead.

Bug: None
Change-Id: I1304eb1cc508d7a76c9a4cbc6954f69a7bcb1637
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1845955Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703712}

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

@@ -513,7 +513,17 @@ BrowserView* BrowserView::GetBrowserViewForNativeWindow(
 
 // static
 BrowserView* BrowserView::GetBrowserViewForBrowser(const Browser* browser) {
-  return static_cast<BrowserView*>(browser->window());
+  // It might look like this method should be implemented as:
+  //   return static_cast<BrowserView*>(browser->window())
+  // but in fact in unit tests browser->window() may not be a BrowserView even
+  // in Views Browser builds. Always go through the ForNativeWindow path, which
+  // is robust against being given any kind of native window.
+  //
+  // Also, tests don't always have a non-null NativeWindow backing the
+  // BrowserWindow, so be sure to check for that as well.
+  if (!browser->window()->GetNativeWindow())
+    return nullptr;
+  return GetBrowserViewForNativeWindow(browser->window()->GetNativeWindow());
 }
 
 // static

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

@@ -20,6 +20,7 @@
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
+#include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/version_info/channel.h"
 #include "content/public/test/test_service_manager_context.h"
@@ -354,3 +355,12 @@ TEST_F(BrowserViewHostedAppTest, Layout) {
   EXPECT_EQ(browser_view()->GetFindBarBoundingBox().y(),
             browser_view()->frame()->GetTopInset());
 }
+
+using BrowserViewWindowTypeTest = BrowserWithTestWindowTest;
+
+TEST_F(BrowserViewWindowTypeTest, TestWindowIsNotReturned) {
+  // Check that BrowserView::GetBrowserViewForBrowser does not return a
+  // non-BrowserView BrowserWindow instance - in this case, a TestBrowserWindow.
+  EXPECT_NE(nullptr, browser()->window());
+  EXPECT_EQ(nullptr, BrowserView::GetBrowserViewForBrowser(browser()));
+}