Skip to content

Commit

Permalink
Fixes seg fault crash of BraveViewsDelegateViewLinuxBrowserTest.GetDe…
Browse files Browse the repository at this point in the history
…faultWindowIconTest

As @simonhong noted in his comments: "In test, our BraveViewsDelegateLinux isn't set
because ChromeViewsDelegate is set at the very early state by AccessibilityChecker
(subclass of ChromeViewsDelegate) in ctor of InProcessBrowserTest. Introduced in v68.
To have a test, explicitly set views delegate by initializing here.
This is only effective in release mode because initializing views delegate twice isn't
allowed by dcheck."

In c76 the crash happens due to us initializing BraveViewsDelegateLinux in the test.
To work around it, override AccessibilityChecker to use BraveViewsDelegateLinux when
building browser tests on linux. This also allows to run the test in debug(non-official)
build.
  • Loading branch information
mkarolin committed Jul 10, 2019
1 parent 7538d3c commit 99a4fbb
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 13 deletions.
13 changes: 2 additions & 11 deletions browser/ui/views/brave_views_delegate_linux_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,8 @@

using BraveViewsDelegateViewLinuxBrowserTest = InProcessBrowserTest;

IN_PROC_BROWSER_TEST_F(BraveViewsDelegateViewLinuxBrowserTest, GetDefaultWindowIconTest) {
// In test, our BraveViewsDelegateLinux isn't set because ChromeViewsDelegate is set
// at the very early state by AccessibilityChecker(subclass of ChromeViewsDelegate)
// in ctor of InProcessBrowserTest. Introduced in v68.
// To have a test, explicitly set views delegate by initializing here.
// This is only effective in release mode because initializing views delegate twice isn't
// allowed by dcheck.
BraveViewsDelegateLinux brave_views_delegate;

IN_PROC_BROWSER_TEST_F(BraveViewsDelegateViewLinuxBrowserTest,
GetDefaultWindowIconTest) {
auto env = base::Environment::Create();
views::ViewsDelegate* views_delegate = views::ViewsDelegate::GetInstance();
auto& rb = ui::ResourceBundle::GetSharedInstance();
Expand All @@ -41,9 +34,7 @@ IN_PROC_BROWSER_TEST_F(BraveViewsDelegateViewLinuxBrowserTest, GetDefaultWindowI
EXPECT_EQ(rb.GetImageSkiaNamed(IDR_PRODUCT_LOGO_128_NIGHTLY)->bitmap(),
views_delegate->GetDefaultWindowIcon()->bitmap());
#else // OFFICIAL_BUILD
// TODO(simonhong): Enable this test in debug mode.
EXPECT_EQ(rb.GetImageSkiaNamed(IDR_PRODUCT_LOGO_128_DEVELOPMENT)->bitmap(),
views_delegate->GetDefaultWindowIcon()->bitmap());
#endif
}

20 changes: 20 additions & 0 deletions chromium_src/chrome/test/views/accessibility_checker.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_CHROMIUM_SRC_CHROME_TEST_VIEWS_ACCESSIBILITY_CHECKER_H_
#define BRAVE_CHROMIUM_SRC_CHROME_TEST_VIEWS_ACCESSIBILITY_CHECKER_H_

#include "chrome/browser/ui/views/chrome_views_delegate.h"

#if defined(OS_LINUX)
#include "brave/browser/ui/views/brave_views_delegate_linux.h"
#define ChromeViewsDelegate BraveViewsDelegateLinux
#endif
#include "../../../../../chrome/test/views/accessibility_checker.h"
#if defined(OS_LINUX)
#undef ChromeViewsDelegate
#endif

#endif // BRAVE_CHROMIUM_SRC_CHROME_TEST_VIEWS_ACCESSIBILITY_CHECKER_H_
3 changes: 1 addition & 2 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -442,9 +442,8 @@ test("brave_browser_tests") {
"//chrome/browser/extensions/extension_apitest.h",
]

if (is_linux && is_official_build) {
if (is_linux) {
sources += [
# BraveViewsDelegateLinuxBrowserTest only works in release(official) build.
"//brave/browser/ui/views/brave_views_delegate_linux_browsertest.cc",
]
configs += [
Expand Down

0 comments on commit 99a4fbb

Please sign in to comment.