[iOS Clean] Reenable TabCoordinator unittests. |
|||
Issue descriptionThe TabCoordinator uses the location bar, which has code that checks the current WebThread. Currently, tab_coordinator_unittest.mm attempts to satisfy this check by using a base::MessageLoop and web::TestWebThread, but this causes another DCHECK when trying to create the message loop.
,
Aug 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d65ba5faf97ad45fe61ad1f0909bebf470cf0e62 commit d65ba5faf97ad45fe61ad1f0909bebf470cf0e62 Author: Kurt Horimoto <kkhorimoto@chromium.org> Date: Tue Aug 29 15:55:12 2017 [iOS Clean] Disable TabCoordinator unittests. These tests DHCECK when attempting to create the MessageLoop. Bug: 759761 Change-Id: Ibf65b4363560810f99cc097bd96e0b05c3a8f5c8 Reviewed-on: https://chromium-review.googlesource.com/638837 Reviewed-by: Kurt Horimoto <kkhorimoto@chromium.org> Reviewed-by: edchin <edchin@chromium.org> Commit-Queue: edchin <edchin@chromium.org> Cr-Commit-Position: refs/heads/master@{#498122} [modify] https://crrev.com/d65ba5faf97ad45fe61ad1f0909bebf470cf0e62/ios/clean/chrome/browser/ui/tab/tab_coordinator_unittest.mm
,
Sep 18 2017
[ RUN ] TabCoordinatorTest.DefaultTabContainer [86163:1027:0918/084721.353860:678148946068394:FATAL:chrome_browser_state.mm(82)] Check failed: ::web::WebThread::CurrentlyOn(web::WebThread::UI). Must be called on Web_UIThread; actually called on Unknown Thread. 0 ios_clean_chrome_unittests 0x000000010d8a0e4d base::debug::StackTrace::StackTrace(unsigned long) + 157 1 ios_clean_chrome_unittests 0x000000010d8a0e8d base::debug::StackTrace::StackTrace(unsigned long) + 29 2 ios_clean_chrome_unittests 0x000000010d89f94c base::debug::StackTrace::StackTrace() + 28 3 ios_clean_chrome_unittests 0x000000010d905cdf logging::LogMessage::~LogMessage() + 479 4 ios_clean_chrome_unittests 0x000000010d903835 logging::LogMessage::~LogMessage() + 21 5 ios_clean_chrome_unittests 0x000000010c68ea27 ios::ChromeBrowserState::GetRequestContext() + 343 6 ios_clean_chrome_unittests 0x000000010cdbd9aa OmniboxPopupViewIOS::OmniboxPopupViewIOS(ios::ChromeBrowserState*, OmniboxEditModel*, OmniboxPopupViewSuggestionsDelegate*, id<OmniboxPopupPositioner>) + 1306 7 ios_clean_chrome_unittests 0x000000010cdbeeb5 OmniboxPopupViewIOS::OmniboxPopupViewIOS(ios::ChromeBrowserState*, OmniboxEditModel*, OmniboxPopupViewSuggestionsDelegate*, id<OmniboxPopupPositioner>) + 69 8 ios_clean_chrome_unittests 0x000000010cdcf0b2 decltype(std::make_unique<OmniboxPopupViewIOS>(std::forward<ios::ChromeBrowserState*, OmniboxEditModel*, OmniboxViewIOS*, id<OmniboxPopupPositioner> __strong&>(fp))) base::MakeUnique<OmniboxPopupViewIOS, ios::ChromeBrowserState*, OmniboxEditModel*, OmniboxViewIOS*, id<OmniboxPopupPositioner> __strong&>(OmniboxPopupViewIOS&&) + 354 9 ios_clean_chrome_unittests 0x000000010cdcdf66 OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS*, WebOmniboxEditController*, ios::ChromeBrowserState*, id<PreloadProvider>, id<OmniboxPopupPositioner>) + 2166 10 ios_clean_chrome_unittests 0x000000010cdcf394 OmniboxViewIOS::OmniboxViewIOS(OmniboxTextFieldIOS*, WebOmniboxEditController*, ios::ChromeBrowserState*, id<PreloadProvider>, id<OmniboxPopupPositioner>) + 132 11 ios_clean_chrome_unittests 0x000000010cdae73a decltype(std::make_unique<OmniboxViewIOS>(std::forward<OmniboxTextFieldIOS* __strong&, LocationBarControllerImpl*, ios::ChromeBrowserState*&, id<PreloadProvider> __strong&, id<OmniboxPopupPositioner> __strong&>(fp))) base::MakeUnique<OmniboxViewIOS, OmniboxTextFieldIOS* __strong&, LocationBarControllerImpl*, ios::ChromeBrowserState*&, id<PreloadProvider> __strong&, id<OmniboxPopupPositioner> __strong&>(id<OmniboxPopupPositioner> __strong&&&) + 346 12 ios_clean_chrome_unittests 0x000000010cdae0e7 LocationBarControllerImpl::LocationBarControllerImpl(OmniboxTextFieldIOS*, ios::ChromeBrowserState*, id<PreloadProvider>, id<OmniboxPopupPositioner>, id<LocationBarDelegate>, id<BrowserCommands>) + 391 13 ios_clean_chrome_unittests 0x000000010cdaf47b LocationBarControllerImpl::LocationBarControllerImpl(OmniboxTextFieldIOS*, ios::ChromeBrowserState*, id<PreloadProvider>, id<OmniboxPopupPositioner>, id<LocationBarDelegate>, id<BrowserCommands>) + 187 14 ios_clean_chrome_unittests 0x000000010d841e4d decltype(std::make_unique<LocationBarControllerImpl>(std::forward<OmniboxTextFieldIOS* __strong, ios::ChromeBrowserState*, std::nullptr_t, std::nullptr_t, LocationBarMediator* __strong, std::nullptr_t>(fp))) base::MakeUnique<LocationBarControllerImpl, OmniboxTextFieldIOS* __strong, ios::ChromeBrowserState*, std::nullptr_t, std::nullptr_t, LocationBarMediator* __strong, std::nullptr_t>(OmniboxTextFieldIOS* __strong&&, ios::ChromeBrowserState*&&, std::nullptr_t&&, std::nullptr_t&&, LocationBarMediator* __strong&&, std::nullptr_t&&) + 381 15 ios_clean_chrome_unittests 0x000000010d841208 -[LocationBarCoordinator start] + 1096 16 ios_clean_chrome_unittests 0x000000010d829462 -[ToolbarCoordinator start] + 1938 17 ios_clean_chrome_unittests 0x000000010d7f5fd4 -[TabCoordinator start] + 3940
,
Sep 18 2017
From emails with fdoray@ It is likely that you are trying to instantiate two of these classes in the same scope: base::MessageLoop, base::test::ScopedTaskEnvironment, web::WebThreadBundle. You can only instantiate one of this class in a scope. Use base::MessageLoop if you only need to run tasks on the main thread. Use base::test::ScopedTaskEnvironment if you need to run tasks on the main thread and post tasks via base/task_scheduler/post_task.h. Use web::WebThreadBundle if you need to run tasks on the main thread, post tasks via base/task_scheduler/post_task.h and post tasks via ios/web/public/web_thread.h. To resolve the "Check failed: ::web::WebThread::CurrentlyOn(web::WebThread::UI). Must be called on Web_UIThread; actually called on Unknown Thread." error that you shared with me by email, you would need to add a TestWebThreadBundle to your test and make sure that the code with a DCHECK_CURRENTLY_ON(web::WebThread::UI) runs on the main thread.
,
Sep 19 2017
Stack trace after using web::TestWebThreadBundle thread_bundle_; [98594:1027:0919/150112.791526:762643349185799:FATAL:message_loop.cc(304)] Check failed: !current(). should only have one message loop per thread 0 ios_clean_chrome_unittests 0x000000010a53de4d base::debug::StackTrace::StackTrace(unsigned long) + 157 1 ios_clean_chrome_unittests 0x000000010a53de8d base::debug::StackTrace::StackTrace(unsigned long) + 29 2 ios_clean_chrome_unittests 0x000000010a53c94c base::debug::StackTrace::StackTrace() + 28 3 ios_clean_chrome_unittests 0x000000010a5a2cdf logging::LogMessage::~LogMessage() + 479 4 ios_clean_chrome_unittests 0x000000010a5a0835 logging::LogMessage::~LogMessage() + 21 5 ios_clean_chrome_unittests 0x000000010a5c7828 base::MessageLoop::BindToCurrentThread() + 1928 6 ios_clean_chrome_unittests 0x000000010a5c7d0b base::MessageLoop::MessageLoop(base::MessageLoop::Type) + 75 7 ios_clean_chrome_unittests 0x000000010a8b3d4a base::test::ScopedTaskEnvironment::ScopedTaskEnvironment(base::test::ScopedTaskEnvironment::MainThreadType, base::test::ScopedTaskEnvironment::ExecutionMode) + 154 8 ios_clean_chrome_unittests 0x000000010a8b4951 base::test::ScopedTaskEnvironment::ScopedTaskEnvironment(base::test::ScopedTaskEnvironment::MainThreadType, base::test::ScopedTaskEnvironment::ExecutionMode) + 33 9 ios_clean_chrome_unittests 0x0000000109981cc8 decltype(std::make_unique<base::test::ScopedTaskEnvironment>(std::forward<base::test::ScopedTaskEnvironment::MainThreadType>(fp))) base::MakeUnique<base::test::ScopedTaskEnvironment, base::test::ScopedTaskEnvironment::MainThreadType>(base::test::ScopedTaskEnvironment::MainThreadType&&) + 88 10 ios_clean_chrome_unittests 0x00000001099807b0 web::TestWebThreadBundle::Init(int) + 96 11 ios_clean_chrome_unittests 0x000000010998073f web::TestWebThreadBundle::TestWebThreadBundle() + 479 12 ios_clean_chrome_unittests 0x00000001099810d5 web::TestWebThreadBundle::TestWebThreadBundle() + 21 13 ios_clean_chrome_unittests 0x0000000108c760eb (anonymous namespace)::TabCoordinatorTest::TabCoordinatorTest() + 75 14 ios_clean_chrome_unittests 0x0000000108c7607f TabCoordinatorTest_DefaultTabContainer_Test::TabCoordinatorTest_DefaultTabContainer_Test() + 31 15 ios_clean_chrome_unittests 0x0000000108c76055 TabCoordinatorTest_DefaultTabContainer_Test::TabCoordinatorTest_DefaultTabContainer_Test() + 21
,
Sep 20 2017
A fix for this is in-flight.
,
Sep 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7 commit 5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7 Author: edchin <edchin@chromium.org> Date: Wed Sep 20 22:36:28 2017 [ios] Fix threading issue in tab_coordinator_unittest Code in chrome_browser_state.mm relies on named threads. This CL replaces use of base::test::ScopedTaskEnvironment in BrowserCoordinatorTest class with web::TestWebThreadBundle so that named threads can be used by legacy code. Bug: 759761 Change-Id: Ib4769ccdf66c0270e1f289dd5db2d78a5a9974f3 Reviewed-on: https://chromium-review.googlesource.com/674383 Commit-Queue: edchin <edchin@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Mark Cogan <marq@chromium.org> Reviewed-by: Rohit Rao (ping after 24h) <rohitrao@chromium.org> Cr-Commit-Position: refs/heads/master@{#503273} [modify] https://crrev.com/5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7/ios/chrome/browser/ui/coordinators/BUILD.gn [modify] https://crrev.com/5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7/ios/chrome/browser/ui/coordinators/browser_coordinator_test.h [modify] https://crrev.com/5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7/ios/clean/chrome/browser/ui/tab/tab_coordinator.mm [modify] https://crrev.com/5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7/ios/clean/chrome/browser/ui/tab/tab_coordinator_unittest.mm [modify] https://crrev.com/5fd1d5bcaa1f6fc95c1b409b439a237f50544ed7/ios/clean/chrome/browser/ui/toolbar/toolbar_coordinator.mm
,
Jan 2 2018
Clean is now deprecated. |
|||
►
Sign in to add a comment |
|||
Comment 1 by pkl@chromium.org
, Aug 28 2017Labels: ios-clean-skeleton