Full log: https://build.chromium.org/p/chromium.fyi/builders/UBSanVptr%20Linux/builds/408/steps/views_unittests/logs/stdio

Reproduced:

$ gn gen //out/gn-vptr-null '--args=is_ubsan_vptr=true is_ubsan_no_recover=true is_ubsan_null=true is_debug=false is_component_build=false symbol_level=1 dcheck_always_on=true' --check
$ ninja -C out/gn-vptr-null views_unittests
$ ./out/gn-vptr-null/views_unittests --gtest_filter=FocusManagerTest.AdvanceFocusStaysInWidget
...
[ RUN      ] FocusManagerTest.AdvanceFocusStaysInWidget
../../ui/views/focus/focus_manager.cc:520:33: runtime error: member call on null pointer of type 'views::View'
    #0 0x165a7ed in views::FocusManager::ViewRemoved(views::View*) out/gn-vptr-null/../../ui/views/focus/focus_manager.cc:520:33
    #1 0x16ba2de in views::Widget::OnNativeWidgetDestroying() out/gn-vptr-null/../../ui/views/widget/widget.cc:1072:24
    #2 0x1715a69 in views::NativeWidgetAura::OnWindowDestroying(aura::Window*) out/gn-vptr-null/../../ui/views/widget/native_widget_aura.cc:846:14
    #3 0x14747bd in aura::Window::~Window() out/gn-vptr-null/../../ui/aura/window.cc:111:16
    #4 0x1476964 in aura::Window::~Window() out/gn-vptr-null/../../ui/aura/window.cc:104:19
    #5 0x1718f90 in views::NativeWidgetAura::~NativeWidgetAura() out/gn-vptr-null/../../ui/views/widget/native_widget_aura.cc:990:5
    #6 0x17194f4 in views::NativeWidgetAura::~NativeWidgetAura() out/gn-vptr-null/../../ui/views/widget/native_widget_aura.cc:985:39
    #7 0x16ac6af in views::Widget::~Widget() out/gn-vptr-null/../../ui/views/widget/widget.cc:174:5
    #8 0x7c538c in views::FocusManagerTest_AdvanceFocusStaysInWidget_Test::TestBody() out/gn-vptr-null/../../ui/views/focus/focus_manager_unittest.cc:915:1
    #9 0x1fdbe07 in testing::Test::Run() out/gn-vptr-null/../../testing/gtest/src/gtest.cc:2474:5
    #10 0x1fdd45e in testing::TestInfo::Run() out/gn-vptr-null/../../testing/gtest/src/gtest.cc:2656:11
    #11 0x1fde5b7 in testing::TestCase::Run() out/gn-vptr-null/../../testing/gtest/src/gtest.cc:2774:28
    #12 0x1feb957 in testing::internal::UnitTestImpl::RunAllTests() out/gn-vptr-null/../../testing/gtest/src/gtest.cc:4647:43
    #13 0x1fead74 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*
) out/gn-vptr-null/../../testing/gtest/src/gtest.cc:2458:12
    #14 0x1feab0c in testing::UnitTest::Run() out/gn-vptr-null/../../testing/gtest/src/gtest.cc:4255:10
    #15 0xa456fa in RUN_ALL_TESTS out/gn-vptr-null/../../testing/gtest/include/gtest/gtest.h:2237:46
    #16 0xa456fa in base::TestSuite::Run() out/gn-vptr-null/../../base/test/test_suite.cc:230
    #17 0xa58ac3 in Run out/gn-vptr-null/../../base/callback.h:397:12
    #18 0xa58ac3 in base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int (), (base::internal::CopyMode)1> const&, int, int, bool, base::Callback<void (), (base::internal::CopyMode)1> const&)
out/gn-vptr-null/../../base/test/launcher/unit_test_launcher.cc:206
    #19 0xa58948 in base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1> const&) out/gn-vptr-null/../../base/test/launcher/unit_test_launcher.cc:445:10
    #20 0x8d8468 in views::ViewsTestSuite::RunTests() out/gn-vptr-null/../../ui/views/views_test_suite.cc:32:10
    #21 0x819e1a in main out/gn-vptr-null/../../ui/views/run_all_unittests_main.cc:8:44
    #22 0x7f3d795def44 in __libc_start_main /build/eglibc-oGUzwX/eglibc-2.19/csu/libc-start.c:287
    #23 0x495590 in _start (/usr/local/google/home/krasin/chr19/src/out/gn-vptr-null/views_unittests+0x495590)

[1/1] FocusManagerTest.AdvanceFocusStaysInWidget (CRASHED)

So, we got removed == NULL here:
https://cs.chromium.org/chromium/src/ui/views/focus/focus_manager.cc?q=ui/views/focus/focus_manager.cc:520&sq=package:chromium&l=520

void FocusManager::ViewRemoved(View* removed) {
  // If the view being removed contains (or is) the focused view,
  // clear the focus.  However, it's not safe to call ClearFocus()
  // (and in turn ClearNativeFocus()) here because ViewRemoved() can
  // be called while the top level widget is being destroyed.
  if (focused_view_ && removed->Contains(focused_view_))
    SetFocusedView(NULL);
}

Taking a closer look...

This is not a recent regression, and it's not clear to me if we can guarantee removed != NULL. Speculative fix: https://codereview.chromium.org/2036193003

I don't particularly like the fix as may mask a bigger problem. Leaving to the code owners to decide.

The fix (as I suspected) is the right. The sky's comment from the review:

"""
This code shouldn't have to check for a null supplied view. This code should
have a DCHECK(removed) and callers shouldn't supply null.
"""

Given that I have zero knowledge of that code, tentatively reassigning the bug to sky, but I expect that the proper owner for the bug is someone else. Please, help me to find them.

So, I was curious and made a simple change that adds the DCHECK into  FocusManager::ViewRemoved. Then I ran trybots on it to see if passing a NULL into this function is an isolated case or a norm:

https://codereview.chromium.org/2047083002/

It seems to be a norm. Almost all tests which deal with views are affected on all (or almost all) OSes.

Scott, do you have a suggestion how to proceed here? If we're willing to accept the fact it happens, my change (https://codereview.chromium.org/2036193003/) should be the one. If it needs to be fixed, do you have an idea how to do it, or do you know a better owner for the bug?

oh, actually, it's more interesting than that. While it's a norm for NULL to be passed into FocusManager::ViewRemoved, it's only if focused_view_ is also NULL, thus the call is not happening.

I've updated my exploratory CL to test that and found out that FocusManagerTest.AdvanceFocusStaysInWidget is realy special:
https://codereview.chromium.org/2047083002/

The possible second candidate is ViewTest.HandleAccelerator which only fails under Chrome OS:
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/224938

ViewTest.HandleAccelerator really has some Chrome OS specific code:
https://cs.chromium.org/chromium/src/ui/views/view_unittest.cc?q=HandleAccelerator+viewtest&sq=package:chromium&dr=CSs&l=2129

#if defined(USE_AURA) && !defined(OS_CHROMEOS)
  // When a non-child view is not active, it shouldn't handle accelerators.
  EXPECT_FALSE(widget->IsActive());
  EXPECT_FALSE(focus_manager->ProcessAccelerator(return_accelerator));
  EXPECT_EQ(0, view->accelerator_count_map_[return_accelerator]);
#endif

...

#if defined(USE_AURA) && !defined(OS_CHROMEOS)
  // When a child view is not in focus, its parent should handle accelerators.
  child_view->accelerator_count_map_[return_accelerator] = 0;
  view->accelerator_count_map_[return_accelerator] = 0;
  child_focus_manager->ClearFocus();
  EXPECT_FALSE(child_view->GetWidget()->IsActive());
  EXPECT_TRUE(child_focus_manager->ProcessAccelerator(return_accelerator));
  EXPECT_EQ(0, child_view->accelerator_count_map_[return_accelerator]);
  EXPECT_EQ(1, view->accelerator_count_map_[return_accelerator]);
#endif

which deals with the focus. So, not completely a surprise.

FocusManagerTest.AdvanceFocusStaysInWidget failure is related to the destruction of the widget after those two lines:
https://cs.chromium.org/chromium/src/ui/views/focus/focus_manager_unittest.cc?q=AdvanceFocusStaysInWidget&sq=package:chromium&l=912

delegate->set_should_advance_focus_to_parent(true);
GetFocusManager()->AdvanceFocus(true);

Worth noting that set_should_advance_focus_to_parent method is only used once across the whole Chrome codebase: in this test. Unless there's a code outside of what the code search can find that uses it, we may consider deleting this method altogether.

This does not explain the mystery, and it also does not explain ViewTest.HandleAccelerator failure, so I will keep digging.

This fix handles both failures: https://codereview.chromium.org/2047083002/
It's a bit better than a previous one and also adds a DCHECK to warn us, if this happens again.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f785a8c6d9e355eef6ea0b09c6df1eaedc850f4

commit 3f785a8c6d9e355eef6ea0b09c6df1eaedc850f4
Author: krasin <krasin@google.com>
Date: Thu Jun 16 16:45:22 2016

Do not allow NULL to be passed into FocusManager::ViewRemoved, when focused_view_ != NULL.

This could happen when a top view is focused and it's being destroyed.

BUG= 617305 

Review-Url: https://codereview.chromium.org/2047083002
Cr-Commit-Position: refs/heads/master@{#400174}

[modify] https://crrev.com/3f785a8c6d9e355eef6ea0b09c6df1eaedc850f4/ui/views/focus/focus_manager.cc
[modify] https://crrev.com/3f785a8c6d9e355eef6ea0b09c6df1eaedc850f4/ui/views/widget/widget.cc

UBSan Vptr bot is now green!
https://build.chromium.org/p/chromium.fyi/builders/UBSanVptr%20Linux/builds/529