Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
486063002:
MacViews: Fix WebDialogBrowserTest.SizeWindow to get browser_tests compiling on MacViews (Closed)
|
Created:
6 years, 4 months ago by tapted Modified:
6 years, 4 months ago Reviewers:
msw CC:
chromium-reviews, tfarina, chrome-apps-syd-reviews_chromium.org, mac-views-reviews_chromium.org, Scott Byer Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionMacViews: Fix WebDialogBrowserTest.SizeWindow to get browser_tests compiling on MacViews
gyp changes are made to filter out toolkit-views browser_tests that
aren't yet compiled in to a toolkit-views Chrome binary on Mac.
One, WebDialogBrowserTest.SizeWindow, was a disabled test (disabled for
4 years :o). It was preventing browser_tests compiling on MacViews
because it was passing a gfx::NativeWindow to CreateWindowWithParent,
which takes a view.
The parent isn't important for the test - it's just adding widget
context. This CL changes the parent to be web_contents->GetNativeView()
instead so the test compiles.
To ensure nothing breaks, the test needs to be enabled. According to the
comment, the reasons for it being disabled on Windows still seem
relevant. However, on Linux the test was just timing out due to a quirk
of the test. So the quirk is fixed, and WebDialogBrowserTest.SizeWindow
is enabled on Linux Aura.
The test is also enabled on Mac, but currently fails because
NativeViewHost isn't implemented yet. That's coming, and leaving the
test enabled will ensure we check it.
With this change browser_tests compiles and links on toolkit-views Mac.
BUG=404979, 399191, 52602
TEST=browser_tests
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290998
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291272
Patch Set 1 #Patch Set 2 : Submit some jobs with the test enabled #Patch Set 3 : Enable the test in some places #Patch Set 4 : Just parent of native view #Patch Set 5 : fix bug link. nit comments #
Total comments: 5
Patch Set 6 : respond to comments #
Total comments: 2
Patch Set 7 : fix bug link #Patch Set 8 : fix memory leak #
Messages
Total messages: 22 (0 generated)
Hi Michael, please take a look.
https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... File chrome/browser/ui/views/web_dialog_view_browsertest.cc (right): https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... chrome/browser/ui/views/web_dialog_view_browsertest.cc:90: #if defined(OS_WIN) || defined(OS_CHROMEOS) There are some bot runs in crrev.com/487143002. As per the comment, the test isn't happy on Windows (seems content on CrOS though..)
https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... File chrome/browser/ui/views/web_dialog_view_browsertest.cc (right): https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... chrome/browser/ui/views/web_dialog_view_browsertest.cc:90: #if defined(OS_WIN) || defined(OS_CHROMEOS) On 2014/08/19 13:17:57, tapted wrote: > There are some bot runs in crrev.com/487143002. As per the comment, the test > isn't happy on Windows (seems content on CrOS though..) Go ahead and enable on CrOS, that comment is surely out of date. https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... chrome/browser/ui/views/web_dialog_view_browsertest.cc:166: // minimum size to ensure there's something to change. Is this verifying that re-sizing to 0x0 enforces the minimum size? Is there a good reason to add the extra 250 step without verifying that there is indeed a change between that and the 0x0 step? Could you clarify the comment a bit? Consider something like: "Expand beyond the minimum size to ..., then verify that attempts to re-size to 0x0 enforces the minimum size."
https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... File chrome/browser/ui/views/web_dialog_view_browsertest.cc (right): https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... chrome/browser/ui/views/web_dialog_view_browsertest.cc:90: #if defined(OS_WIN) || defined(OS_CHROMEOS) On 2014/08/19 18:56:36, msw wrote: > On 2014/08/19 13:17:57, tapted wrote: > > There are some bot runs in crrev.com/487143002. As per the comment, the test > > isn't happy on Windows (seems content on CrOS though..) > > Go ahead and enable on CrOS, that comment is surely out of date. Done. https://codereview.chromium.org/486063002/diff/120001/chrome/browser/ui/views... chrome/browser/ui/views/web_dialog_view_browsertest.cc:166: // minimum size to ensure there's something to change. On 2014/08/19 18:56:36, msw wrote: > Is this verifying that re-sizing to 0x0 enforces the minimum size? Yes - the old code was doing that below, but it was already at the minimum size at this point. So, not only is SaveWindowPlacement() skipped, but the override in the test harness above explicitly skips posting the QuitClosure if the bounds haven't changed. > Is there a > good reason to add the extra 250 step without verifying that there is indeed a > change between that and the 0x0 step? No harm in verifying (I just skipped it here for being redundant). Added a check. > Could you clarify the comment a bit? > Consider something like: "Expand beyond the minimum size to ..., then verify > that attempts to re-size to 0x0 enforces the minimum size." Done.
LGTM (with question about WontFix'ed bug/comment), you might want to update the CL description to mention the exclusion of various views browser test sources from OS=="mac". https://codereview.chromium.org/486063002/diff/160001/chrome/chrome_tests.gypi File chrome/chrome_tests.gypi (right): https://codereview.chromium.org/486063002/diff/160001/chrome/chrome_tests.gyp... chrome/chrome_tests.gypi:1954: # respective implementations are linked in. http://crbug.com/404980. Did you meant to mark that bug WontFix? Seems odd to mention it here if so.
Thanks Michael! On 2014/08/20 19:51:48, msw wrote: > you might want to update the > CL description to mention the exclusion of various views browser test sources > from OS=="mac". Done. https://codereview.chromium.org/486063002/diff/160001/chrome/chrome_tests.gypi File chrome/chrome_tests.gypi (right): https://codereview.chromium.org/486063002/diff/160001/chrome/chrome_tests.gyp... chrome/chrome_tests.gypi:1954: # respective implementations are linked in. http://crbug.com/404980. On 2014/08/20 19:51:48, msw wrote: > Did you meant to mark that bug WontFix? Seems odd to mention it here if so. nice catch - Thanks! I discovered crbug/I had simultaneously made two identical bugs, and WontFixed the second one. But forgot I'd referenced the second one here :/. Fixed.
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tapted@chromium.org/486063002/180001
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) android_chromium_gn_compile_rel on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...) android_clang_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...) linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) mac_chromium_rel_swarming on tryserver.chromium.mac (http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...) android_clang_dbg on tryserver.chromium.linux (http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tapted@chromium.org/486063002/180001
Message was sent while issue was closed.
Committed patchset #7 (180001) as 290998
Message was sent while issue was closed.
A revert of this CL (patchset #7) has been created in https://codereview.chromium.org/490123002/ by peria@chromium.org. The reason for reverting is: browser_test fails in Linux ASAN/LSAN test. WebDialogBrowserTest.SizeWindow (run #1): [ RUN ] WebDialogBrowserTest.SizeWindow [30383:30383:0820/232516:ERROR:browser_main_loop.cc(162)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on. Xlib: extension "RANDR" missing on display ":9". Xlib: extension "RANDR" missing on display ":9". [30383:30383:0820/232516:WARNING:password_store_factory.cc(215)] Using basic (unencrypted) store for password storage. See http://code.google.com/p/chromium/wiki/LinuxPasswordStorage for more information about password storage options. [30473:30473:0820/232516:ERROR:renderer_main.cc(204)] Running without renderer sandbox [30496:30496:0820/232516:ERROR:renderer_main.cc(204)] Running without renderer sandbox ================================================================= ==30383==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 1 object(s) allocated from: #0 0x4f2bbb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__3_/build/src/out/Release/browser_tests+0x4f2bbb) #1 0x1dace96 in WebDialogBrowserTest_SizeWindow_Test::RunTestOnMainThread() chrome/browser/ui/views/web_dialog_view_browsertest.cc:95:3 #2 0x32aaae3 in InProcessBrowserTest::RunTestOnMainThreadLoop() chrome/test/base/in_process_browser_test.cc:434:5 #3 0x2d6de03 in Run base/callback.h:401:12 #4 0x2d6de03 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:1555 #5 0x2d6a146 in ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:980:18 #6 0x59d80b7 in content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:736:5 #7 0x5d3ac48 in Run base/callback.h:401:12 #8 0x5d3ac48 in content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:45 #9 0x59d40a0 in content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:636:3 #10 0x5f3839d in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner.cc:106:5 #11 0xf90f1a5 in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:22:19 #12 0xf8416c0 in content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:764:12 #13 0xf83db5f in content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:15 #14 0xd6b57e1 in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:257:3 #15 0x32a773b in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:217:3 #16 0x3de9f81 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2418:12 #17 0x3de9f81 in testing::Test::Run() testing/gtest/src/gtest.cc:2430 #18 0x3dec559 in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2610:5 #19 0x3ded5b6 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2728:5 #20 0x3e03055 in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4591:11 #21 0x3e02664 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2418:12 #22 0x3e02664 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4209 #23 0x348e02f in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2304:10 #24 0x348e02f in base::TestSuite::Run() base/test/test_suite.cc:227 #25 0x20fb8cd in ChromeBrowserTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/browser_tests_main.cc:14:12 #26 0xd737de7 in content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:469:12 #27 0x32a4b8b in LaunchChromeTests(int, ChromeTestSuiteRunner*, int, char**) chrome/test/base/chrome_test_launcher.cc:124:10 #28 0x20fb7b8 in main chrome/test/base/browser_tests_main.cc:21:10 #29 0x7feeddc6676c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 ----------------------------------------------------- Suppressions used: count bytes template 1767 75328 libfontconfig ----------------------------------------------------- SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s). [0820/232518:ERROR:nacl_helper_linux.cc(282)] NaCl helper process running without a sandbox! Most likely you need to configure your SUID sandbox correctly WebDialogBrowserTest.SizeWindow (run #2): [ RUN ] WebDialogBrowserTest.SizeWindow [16767:16767:0820/232720:ERROR:browser_main_loop.cc(162)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on. Xlib: extension "RANDR" missing on display ":9". Xlib: extension "RANDR" missing on display ":9". [16767:16767:0820/232720:WARNING:password_store_factory.cc(215)] Using basic (unencrypted) store for password storage. See http://code.google.com/p/chromium/wiki/LinuxPasswordStorage for more information about password storage options. [16871:16871:0820/232720:ERROR:renderer_main.cc(204)] Running without renderer sandbox [16911:16911:0820/232720:ERROR:renderer_main.cc(204)] Running without renderer sandbox ================================================================= ==16767==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 1 object(s) allocated from: #0 0x4f2bbb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__3_/build/src/out/Release/browser_tests+0x4f2bbb) #1 0x1dace96 in WebDialogBrowserTest_SizeWindow_Test::RunTestOnMainThread() chrome/browser/ui/views/web_dialog_view_browsertest.cc:95:3 #2 0x32aaae3 in InProcessBrowserTest::RunTestOnMainThreadLoop() chrome/test/base/in_process_browser_test.cc:434:5 #3 0x2d6de03 in Run base/callback.h:401:12 #4 0x2d6de03 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:1555 #5 0x2d6a146 in ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:980:18 #6 0x59d80b7 in content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:736:5 #7 0x5d3ac48 in Run base/callback.h:401:12 #8 0x5d3ac48 in content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:45 #9 0x59d40a0 in content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:636:3 #10 0x5f3839d in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner.cc:106:5 #11 0xf90f1a5 in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:22:19 #12 0xf8416c0 in content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:764:12 #13 0xf83db5f in content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:15 #14 0xd6b57e1 in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:257:3 #15 0x32a773b in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:217:3 #16 0x3de9f81 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2418:12 #17 0x3de9f81 in testing::Test::Run() testing/gtest/src/gtest.cc:2430 #18 0x3dec559 in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2610:5 #19 0x3ded5b6 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2728:5 #20 0x3e03055 in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4591:11 #21 0x3e02664 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2418:12 #22 0x3e02664 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4209 #23 0x348e02f in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2304:10 #24 0x348e02f in base::TestSuite::Run() base/test/test_suite.cc:227 #25 0x20fb8cd in ChromeBrowserTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/browser_tests_main.cc:14:12 #26 0xd737de7 in content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:469:12 #27 0x32a4b8b in LaunchChromeTests(int, ChromeTestSuiteRunner*, int, char**) chrome/test/base/chrome_test_launcher.cc:124:10 #28 0x20fb7b8 in main chrome/test/base/browser_tests_main.cc:21:10 #29 0x7fb13f28476c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 ----------------------------------------------------- Suppressions used: count bytes template 1767 75328 libfontconfig ----------------------------------------------------- SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s). [0820/232722:ERROR:nacl_helper_linux.cc(282)] NaCl helper process running without a sandbox! Most likely you need to configure your SUID sandbox correctly WebDialogBrowserTest.SizeWindow (run #3): [ RUN ] WebDialogBrowserTest.SizeWindow [16994:16994:0820/232723:ERROR:browser_main_loop.cc(162)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on. Xlib: extension "RANDR" missing on display ":9". Xlib: extension "RANDR" missing on display ":9". [16994:16994:0820/232724:WARNING:password_store_factory.cc(215)] Using basic (unencrypted) store for password storage. See http://code.google.com/p/chromium/wiki/LinuxPasswordStorage for more information about password storage options. [17026:17026:0820/232724:ERROR:renderer_main.cc(204)] Running without renderer sandbox [17038:17038:0820/232724:ERROR:renderer_main.cc(204)] Running without renderer sandbox ================================================================= ==16994==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 1 object(s) allocated from: #0 0x4f2bbb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__3_/build/src/out/Release/browser_tests+0x4f2bbb) #1 0x1dace96 in WebDialogBrowserTest_SizeWindow_Test::RunTestOnMainThread() chrome/browser/ui/views/web_dialog_view_browsertest.cc:95:3 #2 0x32aaae3 in InProcessBrowserTest::RunTestOnMainThreadLoop() chrome/test/base/in_process_browser_test.cc:434:5 #3 0x2d6de03 in Run base/callback.h:401:12 #4 0x2d6de03 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:1555 #5 0x2d6a146 in ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:980:18 #6 0x59d80b7 in content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:736:5 #7 0x5d3ac48 in Run base/callback.h:401:12 #8 0x5d3ac48 in content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:45 #9 0x59d40a0 in content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:636:3 #10 0x5f3839d in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner.cc:106:5 #11 0xf90f1a5 in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:22:19 #12 0xf8416c0 in content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:764:12 #13 0xf83db5f in content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:15 #14 0xd6b57e1 in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:257:3 #15 0x32a773b in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:217:3 #16 0x3de9f81 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2418:12 #17 0x3de9f81 in testing::Test::Run() testing/gtest/src/gtest.cc:2430 #18 0x3dec559 in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2610:5 #19 0x3ded5b6 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2728:5 #20 0x3e03055 in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4591:11 #21 0x3e02664 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2418:12 #22 0x3e02664 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4209 #23 0x348e02f in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2304:10 #24 0x348e02f in base::TestSuite::Run() base/test/test_suite.cc:227 #25 0x20fb8cd in ChromeBrowserTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/browser_tests_main.cc:14:12 #26 0xd737de7 in content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:469:12 #27 0x32a4b8b in LaunchChromeTests(int, ChromeTestSuiteRunner*, int, char**) chrome/test/base/chrome_test_launcher.cc:124:10 #28 0x20fb7b8 in main chrome/test/base/browser_tests_main.cc:21:10 #29 0x7f0688ff376c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 ----------------------------------------------------- Suppressions used: count bytes template 1767 75328 libfontconfig ----------------------------------------------------- SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s). [0820/232725:ERROR:nacl_helper_linux.cc(282)] NaCl helper process running without a sandbox! Most likely you need to configure your SUID sandbox correctly WebDialogBrowserTest.SizeWindow (run #4): [ RUN ] WebDialogBrowserTest.SizeWindow [17073:17073:0820/232726:ERROR:browser_main_loop.cc(162)] Running without the SUID sandbox! See https://code.google.com/p/chromium/wiki/LinuxSUIDSandboxDevelopment for more information on developing with the sandbox on. Xlib: extension "RANDR" missing on display ":9". Xlib: extension "RANDR" missing on display ":9". [17073:17073:0820/232726:WARNING:password_store_factory.cc(215)] Using basic (unencrypted) store for password storage. See http://code.google.com/p/chromium/wiki/LinuxPasswordStorage for more information about password storage options. [17105:17105:0820/232726:ERROR:renderer_main.cc(204)] Running without renderer sandbox [17116:17116:0820/232726:ERROR:renderer_main.cc(204)] Running without renderer sandbox ================================================================= ==17073==ERROR: LeakSanitizer: detected memory leaks Direct leak of 128 byte(s) in 1 object(s) allocated from: #0 0x4f2bbb in operator new(unsigned long) (/b/build/slave/Linux_ASan_LSan_Tests__3_/build/src/out/Release/browser_tests+0x4f2bbb) #1 0x1dace96 in WebDialogBrowserTest_SizeWindow_Test::RunTestOnMainThread() chrome/browser/ui/views/web_dialog_view_browsertest.cc:95:3 #2 0x32aaae3 in InProcessBrowserTest::RunTestOnMainThreadLoop() chrome/test/base/in_process_browser_test.cc:434:5 #3 0x2d6de03 in Run base/callback.h:401:12 #4 0x2d6de03 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() chrome/browser/chrome_browser_main.cc:1555 #5 0x2d6a146 in ChromeBrowserMainParts::PreMainMessageLoopRun() chrome/browser/chrome_browser_main.cc:980:18 #6 0x59d80b7 in content::BrowserMainLoop::PreMainMessageLoopRun() content/browser/browser_main_loop.cc:736:5 #7 0x5d3ac48 in Run base/callback.h:401:12 #8 0x5d3ac48 in content::StartupTaskRunner::RunAllTasksNow() content/browser/startup_task_runner.cc:45 #9 0x59d40a0 in content::BrowserMainLoop::CreateStartupTasks() content/browser/browser_main_loop.cc:636:3 #10 0x5f3839d in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) content/browser/browser_main_runner.cc:106:5 #11 0xf90f1a5 in content::BrowserMain(content::MainFunctionParams const&) content/browser/browser_main.cc:22:19 #12 0xf8416c0 in content::ContentMainRunnerImpl::Run() content/app/content_main_runner.cc:764:12 #13 0xf83db5f in content::ContentMain(content::ContentMainParams const&) content/app/content_main.cc:19:15 #14 0xd6b57e1 in content::BrowserTestBase::SetUp() content/public/test/browser_test_base.cc:257:3 #15 0x32a773b in InProcessBrowserTest::SetUp() chrome/test/base/in_process_browser_test.cc:217:3 #16 0x3de9f81 in HandleExceptionsInMethodIfSupported\u003Ctesting::Test, void> testing/gtest/src/gtest.cc:2418:12 #17 0x3de9f81 in testing::Test::Run() testing/gtest/src/gtest.cc:2430 #18 0x3dec559 in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2610:5 #19 0x3ded5b6 in testing::TestCase::Run() testing/gtest/src/gtest.cc:2728:5 #20 0x3e03055 in testing::internal::UnitTestImpl::RunAllTests() testing/gtest/src/gtest.cc:4591:11 #21 0x3e02664 in HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool> testing/gtest/src/gtest.cc:2418:12 #22 0x3e02664 in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4209 #23 0x348e02f in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2304:10 #24 0x348e02f in base::TestSuite::Run() base/test/test_suite.cc:227 #25 0x20fb8cd in ChromeBrowserTestSuiteRunner::RunTestSuite(int, char**) chrome/test/base/browser_tests_main.cc:14:12 #26 0xd737de7 in content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) content/public/test/test_launcher.cc:469:12 #27 0x32a4b8b in LaunchChromeTests(int, ChromeTestSuiteRunner*, int, char**) chrome/test/base/chrome_test_launcher.cc:124:10 #28 0x20fb7b8 in main chrome/test/base/browser_tests_main.cc:21:10 #29 0x7f5f8410c76c in __libc_start_main /build/buildd/eglibc-2.15/csu/libc-start.c:226 ----------------------------------------------------- Suppressions used: count bytes template 1767 75328 libfontconfig ----------------------------------------------------- SUMMARY: AddressSanitizer: 128 byte(s) leaked in 1 allocation(s). [0820/232728:ERROR:nacl_helper_linux.cc(282)] NaCl helper process running without a sandbox! Most likely you need to configure your SUID sandbox correctly.
Of course... a test disabled for 4 years would have a memory leak in it :/. I think this is the nicest way to fix it. trybots haven't been cooperating (they go purple now :o), but maybe the latest batch will report in.
still lgtm, but re-using the same codereview issue to re-land a patch is discouraged. The common pattern I see is to upload the reverted patch set as a new issue, then upload subsequent patch sets fixing any problems (to ease re-review).
On 2014/08/21 19:34:24, msw wrote: > still lgtm, but re-using the same codereview issue to re-land a patch is > discouraged. The common pattern I see is to upload the reverted patch set as a > new issue, then upload subsequent patch sets fixing any problems (to ease > re-review). Yeah - re-using CLs came up again recently - https://groups.google.com/a/chromium.org/d/msg/infra-dev/UBOakX2ccKY/k3_GxBwE... . My understanding is that if re-use would make the updated CL review hard to follow, then a new CL is better. (This one seems simple enough :). Otherwise, the tools are being designed to support both workflows. I actually find I dig through review comments a lot after a `git cl annotate` (e.g. to ask "why was this code done this way" and found an answer in a review comment) - spreading the comments across multiple CLs would make this slightly more annoying, but I don't really feel strongly about it.
The CQ bit was checked by tapted@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/tapted@chromium.org/486063002/220001
Message was sent while issue was closed.
Committed patchset #8 (220001) as 291272
Message was sent while issue was closed.
On 2014/08/21 23:23:45, tapted wrote: > On 2014/08/21 19:34:24, msw wrote: > > still lgtm, but re-using the same codereview issue to re-land a patch is > > discouraged. The common pattern I see is to upload the reverted patch set as a > > new issue, then upload subsequent patch sets fixing any problems (to ease > > re-review). > > Yeah - re-using CLs came up again recently - > https://groups.google.com/a/chromium.org/d/msg/infra-dev/UBOakX2ccKY/k3_GxBwE... > . My understanding is that if re-use would make the updated CL review hard to > follow, then a new CL is better. (This one seems simple enough :). Otherwise, > the tools are being designed to support both workflows. > > I actually find I dig through review comments a lot after a `git cl annotate` > (e.g. to ask "why was this code done this way" and found an answer in a review > comment) - spreading the comments across multiple CLs would make this slightly > more annoying, but I don't really feel strongly about it. Thanks for the info! I agree reusing issues is simpler for simple fixes. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
