New issue
Advanced search Search tips

Issue 804231 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----



Sign in to add a comment

BackgroundContentsServiceNotificationTest.TestShowBalloon and 3 other(s) in unit_tests failing on chromium.memory/Linux CFI

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Jan 22 2018

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of yoichio@google.com

BackgroundContentsServiceNotificationTest.TestShowBalloon and 3 other(s) in unit_tests failing on chromium.memory/Linux CFI

Builders failed on: 
- Linux CFI: 
  https://build.chromium.org/p/chromium.memory/builders/Linux%20CFI
failures:
BackgroundContentsServiceNotificationTest.TestShowBalloonNoIcon
BackgroundContentsServiceNotificationTest.TestShowBalloonShutdown
BackgroundContentsServiceNotificationTest.TestShowBalloon
BackgroundContentsServiceNotificationTest.TestShowTwoBalloons

Failing started from: 
 https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20CFI/builds/5328


 

Comment 1 by mastiz@chromium.org, Jan 22 2018

Status: Started (was: Available)

Comment 2 by mastiz@chromium.org, Jan 22 2018

Labels: -Pri-2 Pri-1
Owner: est...@chromium.org
Status: Assigned (was: Started)
Culprint seems to be  https://chromium-review.googlesource.com/874572

I assume broken tests are not painful enough to revert the patch, so I'm reassigning to the contributor and bumping prio, thanks.

Comment 3 by est...@chromium.org, Jan 22 2018

Status: Started (was: Assigned)
here is the stack. It should be easy to fix.

[ RUN      ] BackgroundContentsServiceNotificationTest.TestShowBalloonNoIcon
../../components/keyed_service/content/browser_context_keyed_service_factory.cc:118:26: runtime error: control flow integrity check for type 'content::BrowserContext' failed during base-to-derived cast (vtable address 0xffffde15a773f38c)
0xffffde15a773f38c: note: invalid vtable
<memory cannot be printed>
    #0 0xce4490c in BrowserContextKeyedServiceFactory::ContextShutdown(base::SupportsUserData*) components/keyed_service/content/browser_context_keyed_service_factory.cc:118:26
    #1 0xc57caf3 in KeyedServiceFactory::SetTestingFactory(base::SupportsUserData*, std::__1::function<std::__1::unique_ptr<KeyedService, std::__1::default_delete<KeyedService> > (base::SupportsUserData*)>) components/keyed_service/core/keyed_service_factory.cc:42:3
    #2 0xce4421c in BrowserContextKeyedServiceFactory::SetTestingFactory(content::BrowserContext*, std::__1::unique_ptr<KeyedService, std::__1::default_delete<KeyedService> > (*)(content::BrowserContext*)) components/keyed_service/content/browser_context_keyed_service_factory.cc:24:24
    #3 0x679b239 in operator() buildtools/third_party/libc++/trunk/include/memory:2233:5
    #4 0x679b239 in reset buildtools/third_party/libc++/trunk/include/memory:2546:0
    #5 0x679b239 in ~unique_ptr buildtools/third_party/libc++/trunk/include/memory:2500:0
    #6 0x679b239 in BackgroundContentsServiceNotificationTest::~BackgroundContentsServiceNotificationTest() chrome/browser/background/background_contents_service_unittest.cc:121:0
    #7 0x679b26d in BackgroundContentsServiceNotificationTest_TestShowBalloonNoIcon_Test::~BackgroundContentsServiceNotificationTest_TestShowBalloonNoIcon_Test() chrome/browser/background/background_contents_service_unittest.cc:313:1
    #8 0x72cf48b in testing::TestInfo::Run() third_party/googletest/src/googletest/src/gtest.cc:2656:3
    #9 0x72cfca1 in testing::TestCase::Run() third_party/googletest/src/googletest/src/gtest.cc:2769:28
    #10 0x72d51b2 in testing::internal::UnitTestImpl::RunAllTests() third_party/googletest/src/googletest/src/gtest.cc:4665:43
    #11 0x72d4e4c in testing::UnitTest::Run() third_party/googletest/src/googletest/src/gtest.cc:4277:10
    #12 0x9c11fbc in base::TestSuite::Run() base/test/test_suite.cc:272:16
    #13 0x9c07210 in int base::internal::Invoker<base::internal::BindState<int (content::UnitTestTestSuite::*)(), base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int ()>::RunImpl<int (content::UnitTestTestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > const&, 0ul>(int (content::UnitTestTestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> > const&, std::__1::integer_sequence<unsigned long, 0ul>) base/bind_internal.h:368:12
    #14 0x9c1550f in base::(anonymous namespace)::LaunchUnitTestsInternal(base::RepeatingCallback<int ()> const&, unsigned long, int, bool, base::RepeatingCallback<void ()> const&) base/test/launcher/unit_test_launcher.cc:220:27
    #15 0x9c153d8 in base::LaunchUnitTests(int, char**, base::RepeatingCallback<int ()> const&) base/test/launcher/unit_test_launcher.cc:558:10
    #16 0x9c07030 in main chrome/test/base/run_all_unittests.cc:30:10
    #17 0x7f79c0124f44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287:0
    #18 0x5951029 in _start ??:0:0
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 22 2018

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

commit 8914364996749f80ea61b4995db0e1a563bf698e
Author: Evan Stade <estade@chromium.org>
Date: Mon Jan 22 20:52:58 2018

Fix Linux CFI build

Tear down the NotificationDisplayServiceTester before the Profile.

TBR=atwilson@chromium.org

Bug:  804231 
Change-Id: I16e1d27e0de560f18d0ec3433adec9610c91a22c
Reviewed-on: https://chromium-review.googlesource.com/879324
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#530985}
[modify] https://crrev.com/8914364996749f80ea61b4995db0e1a563bf698e/chrome/browser/background/background_contents_service_unittest.cc

Comment 5 by est...@chromium.org, Jan 22 2018

Status: Fixed (was: Started)
should be fixed
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 25 2018

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

commit 7a379f73fa1b62e48659119ecd80d99ed463a641
Author: Evan Stade <estade@chromium.org>
Date: Thu Jan 25 18:16:52 2018

Modify shutdown sequence of NotificationDisplayServiceTester.

This is a more general fix for the CFI issues seen while updating tests
to use NotificationDisplayServiceTester. While referencing |profile_|
after shutdown would typically be sorta safe in this case, the CFI bot
objects because it detects a cast on a destroyed object.

The old solution, administered somewhat inconsistently, was to destroy
the NotificationDisplayServiceTester before the profile. However this
also has issues, because some code references the
NotificationDisplayService during profile shutdown. Hence it is not
safe for NDSTester to outlive its profile, and not safe not to outlive
its profile.

The solution is to allow it to outlive its profile but not reference
the profile after it's been destroyed.

TBR=rdevlin.cronin@chromium.org,atwilson@chromium.org

Bug:  804231 
Change-Id: I17f29befb18f0667b2f15ca2bb7554af1854b6d1
Reviewed-on: https://chromium-review.googlesource.com/883224
Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531950}
[modify] https://crrev.com/7a379f73fa1b62e48659119ecd80d99ed463a641/chrome/browser/background/background_contents_service_unittest.cc
[modify] https://crrev.com/7a379f73fa1b62e48659119ecd80d99ed463a641/chrome/browser/extensions/extension_storage_monitor_browsertest.cc
[modify] https://crrev.com/7a379f73fa1b62e48659119ecd80d99ed463a641/chrome/browser/notifications/notification_display_service_tester.cc
[modify] https://crrev.com/7a379f73fa1b62e48659119ecd80d99ed463a641/chrome/browser/notifications/notification_display_service_tester.h

Sign in to add a comment