Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2613163002:
[ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. (Closed)
|
Created:
3 years, 11 months ago by Eugene But (OOO till 7-30) Modified:
3 years, 11 months ago CC:
chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest.
Use TestJavaScriptDialogPresenter instead of deprecated
CRWWebUserInterfaceDelegate.
BUG=661445
TBR=kkhorimoto@chromium.org
Review-Url: https://codereview.chromium.org/2613163002
Cr-Commit-Position: refs/heads/master@{#441998}
Committed: https://chromium.googlesource.com/chromium/src/+/e3cb6842bb7d440a8f8259bea661e5b97159fdca
Patch Set 1 #
Total comments: 2
Depends on Patchset: Dependent Patchsets: Messages
Total messages: 17 (10 generated)
eugenebut@chromium.org changed reviewers: + michaeldo@chromium.org
The CQ bit was checked by eugenebut@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2613163002/diff/1/ios/web/web_state/ui/crw_we... File ios/web/web_state/ui/crw_web_controller_unittest.mm (left): https://codereview.chromium.org/2613163002/diff/1/ios/web/web_state/ui/crw_we... ios/web/web_state/ui/crw_web_controller_unittest.mm:490: // Mocks CRWWebDelegate object. Should we keep comments on these?
Description was changed from ========== [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. Use TestJavaScriptDialogPresenter instead of deprecated CRWWebUserInterfaceDelegate. BUG=661445 ========== to ========== [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. Use TestJavaScriptDialogPresenter instead of deprecated CRWWebUserInterfaceDelegate. BUG=661445 TBR=kkhorimoto@chromium.org ==========
Thanks! https://codereview.chromium.org/2613163002/diff/1/ios/web/web_state/ui/crw_we... File ios/web/web_state/ui/crw_web_controller_unittest.mm (left): https://codereview.chromium.org/2613163002/diff/1/ios/web/web_state/ui/crw_we... ios/web/web_state/ui/crw_web_controller_unittest.mm:490: // Mocks CRWWebDelegate object. On 2017/01/06 18:24:33, michaeldo wrote: > Should we keep comments on these? I don't think these comments add anything. It is pretty obvious that ui_delegate_mock_ "mocks CRWWebUserInterfaceDelegate object.". Per Style Guide comments are required for non-trivial interfaces, but we used to enforce them for all interfaces, like in this case.
The CQ bit was checked by eugenebut@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1483728876603940, "parent_rev":
"344517411399fc467e5a6938e4793650729b3506", "commit_rev":
"e3cb6842bb7d440a8f8259bea661e5b97159fdca"}
Message was sent while issue was closed.
Description was changed from ========== [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. Use TestJavaScriptDialogPresenter instead of deprecated CRWWebUserInterfaceDelegate. BUG=661445 TBR=kkhorimoto@chromium.org ========== to ========== [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. Use TestJavaScriptDialogPresenter instead of deprecated CRWWebUserInterfaceDelegate. BUG=661445 TBR=kkhorimoto@chromium.org Review-Url: https://codereview.chromium.org/2613163002 Cr-Commit-Position: refs/heads/master@{#441998} Committed: https://chromium.googlesource.com/chromium/src/+/e3cb6842bb7d440a8f8259bea661... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/e3cb6842bb7d440a8f8259bea661...
Message was sent while issue was closed.
rohitrao@chromium.org changed reviewers: + rohitrao@chromium.org
Message was sent while issue was closed.
This is crashing on 32-bit devices. https://uberchromegw.corp.google.com/i/chromium.fyi/builders/ios-simulator/bu... Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.CoreFoundation 0x05b57301 _CFRelease + 1505 1 org.chromium.gtest.generic-unit-test 0x00a3afb7 base::internal::ScopedNSProtocolTraitsRelease(objc_object*) + 39 2 org.chromium.gtest.generic-unit-test 0x00071d47 base::internal::ScopedNSProtocolTraits<NSString*>::Release(NSString*) + 23 3 org.chromium.gtest.generic-unit-test 0x00071d28 base::ScopedTypeRef<NSString*, base::internal::ScopedNSProtocolTraits<NSString*> >::~ScopedTypeRef() + 40 4 org.chromium.gtest.generic-unit-test 0x00071cf7 base::scoped_nsprotocol<NSString*>::~scoped_nsprotocol() + 23 5 org.chromium.gtest.generic-unit-test 0x00071cd7 base::scoped_nsobject<NSString>::~scoped_nsobject() + 23 6 org.chromium.gtest.generic-unit-test 0x00071567 base::scoped_nsobject<NSString>::~scoped_nsobject() + 23 7 org.chromium.gtest.generic-unit-test 0x000716dd web::TestJavaScriptDialog::~TestJavaScriptDialog() + 45 8 org.chromium.gtest.generic-unit-test 0x00071707 web::TestJavaScriptDialog::~TestJavaScriptDialog() + 23 9 org.chromium.gtest.generic-unit-test 0x00383724 (anonymous namespace)::CRWWebControllerPageDialogOpenPolicyTest_AllowConfirmWithTrue_Test::TestBody() + 4212 10 org.chromium.gtest.generic-unit-test 0x00adafe7 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 103 11 org.chromium.gtest.generic-unit-test 0x00aae98a void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 106 12 org.chromium.gtest.generic-unit-test 0x00aae8b6 testing::Test::Run() + 246 13 org.chromium.gtest.generic-unit-test 0x00ab062f testing::TestInfo::Run() + 255 14 org.chromium.gtest.generic-unit-test 0x00ab1d44 testing::TestCase::Run() + 260 15 org.chromium.gtest.generic-unit-test 0x00ac59ce testing::internal::UnitTestImpl::RunAllTests() + 974 16 org.chromium.gtest.generic-unit-test 0x00add3c7 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 103 17 org.chromium.gtest.generic-unit-test 0x00ac559a bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 106 18 org.chromium.gtest.generic-unit-test 0x00ac549f testing::UnitTest::Run() + 335 19 org.chromium.gtest.generic-unit-test 0x00a4bc93 RUN_ALL_TESTS() + 19 20 org.chromium.gtest.generic-unit-test 0x00a4b1b7 base::TestSuite::Run() + 279 21 org.chromium.gtest.generic-unit-test 0x00a4ec4a -[ChromeUnitTestDelegate runTests] + 42 22 com.apple.Foundation 0x068527c1 __NSFireDelayedPerform + 442 23 com.apple.CoreFoundation 0x05aa35d6 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 22 24 com.apple.CoreFoundation 0x05aa30ed __CFRunLoopDoTimer + 1213 25 com.apple.CoreFoundation 0x05aa2bff __CFRunLoopDoTimers + 255 26 com.apple.CoreFoundation 0x05a9a6c0 __CFRunLoopRun + 2208 27 com.apple.CoreFoundation 0x05a99bab CFRunLoopRunSpecific + 395 28 com.apple.CoreFoundation 0x05a99a0b CFRunLoopRunInMode + 123 29 com.apple.GraphicsServices 0x093feb4c GSEventRunModal + 177 30 com.apple.GraphicsServices 0x093fe9c7 GSEventRun + 80 31 com.apple.UIKit 0x042b77fb UIApplicationMain + 148 32 org.chromium.gtest.generic-unit-test 0x00a4efdf base::RunTestsFromIOSApp() + 95 33 org.chromium.gtest.generic-unit-test 0x00a4b0c4 base::TestSuite::Run() + 36 34 org.chromium.gtest.generic-unit-test 0x0006ed28 int base::internal::FunctorTraits<int (base::TestSuite::*)(), void>::Invoke<web::WebTestSuite*>(int (base::TestSuite::*)(), web::WebTestSuite*&&) + 104 35 org.chromium.gtest.generic-unit-test 0x0006ec68 int base::internal::InvokeHelper<false, int>::MakeItSo<int (base::TestSuite::* const&)(), web::WebTestSuite*>(int (base::TestSuite::* const&&&)(), web::WebTestSuite*&&) + 72 36 org.chromium.gtest.generic-unit-test 0x0006ec0c int base::internal::Invoker<base::internal::BindState<int (base::TestSuite::*)(), base::internal::UnretainedWrapper<web::WebTestSuite> >, int ()>::RunImpl<int (base::TestSuite::* const&)(), std::__1::tuple<base::internal::UnretainedWrapper<web::WebTestSuite> > const&, 0ul>(int (base::TestSuite::* const&&&)(), std::__1::tuple<base::internal::UnretainedWrapper<web::WebTestSuite> > const&&&, base::IndexSequence<0ul>) + 76 37 org.chromium.gtest.generic-unit-test 0x0006eb2a base::internal::Invoker<base::internal::BindState<int (base::TestSuite::*)(), base::internal::UnretainedWrapper<web::WebTestSuite> >, int ()>::Run(base::internal::BindStateBase*) + 42 38 org.chromium.gtest.generic-unit-test 0x0110bdd9 base::internal::RunMixin<base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> >::Run() const + 57 39 org.chromium.gtest.generic-unit-test 0x00a51265 base::LaunchUnitTests(int, char**, base::Callback<int (), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&) + 1029 40 org.chromium.gtest.generic-unit-test 0x0006e8a9 main + 169 41 libdyld.dylib 0x0ba6a799 start + 1
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2615283002/ by rohitrao@chromium.org. The reason for reverting is: This is crashing on 32-bit devices. Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.CoreFoundation 0x05b57301 _CFRelease + 1505 1 org.chromium.gtest.generic-unit-test 0x00a3afb7 base::internal::ScopedNSProtocolTraitsRelease(objc_object*) + 39 2 org.chromium.gtest.generic-unit-test 0x00071d47 base::internal::ScopedNSProtocolTraits<NSString*>::Release(NSString*) + 23 3 org.chromium.gtest.generic-unit-test 0x00071d28 base::ScopedTypeRef<NSString*, base::internal::ScopedNSProtocolTraits<NSString*> >::~ScopedTypeRef() + 40 4 org.chromium.gtest.generic-unit-test 0x00071cf7 base::scoped_nsprotocol<NSString*>::~scoped_nsprotocol() + 23 5 org.chromium.gtest.generic-unit-test 0x00071cd7 base::scoped_nsobject<NSString>::~scoped_nsobject() + 23 6 org.chromium.gtest.generic-unit-test 0x00071567 base::scoped_nsobject<NSString>::~scoped_nsobject() + 23 7 org.chromium.gtest.generic-unit-test 0x000716dd web::TestJavaScriptDialog::~TestJavaScriptDialog() + 45 8 org.chromium.gtest.generic-unit-test 0x00071707 web::TestJavaScriptDialog::~TestJavaScriptDialog() + 23 9 org.chromium.gtest.generic-unit-test 0x00383724 (anonymous namespace)::CRWWebControllerPageDialogOpenPolicyTest_AllowConfirmWithTrue_Test::TestBody() + 4212 10 org.chromium.gtest.generic-unit-test 0x00adafe7 void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 103 11 org.chromium.gtest.generic-unit-test 0x00aae98a void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 106 12 org.chromium.gtest.generic-unit-test 0x00aae8b6 testing::Test::Run() + 246 13 org.chromium.gtest.generic-unit-test 0x00ab062f testing::TestInfo::Run() + 255 14 org.chromium.gtest.generic-unit-test 0x00ab1d44 testing::TestCase::Run() + 260 15 org.chromium.gtest.generic-unit-test 0x00ac59ce testing::internal::UnitTestImpl::RunAllTests() + 974 16 org.chromium.gtest.generic-unit-test 0x00add3c7 bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 103 17 org.chromium.gtest.generic-unit-test 0x00ac559a bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) + 106 18 org.chromium.gtest.generic-unit-test 0x00ac549f testing::UnitTest::Run() + 335 19 org.chromium.gtest.generic-unit-test 0x00a4bc93 RUN_ALL_TESTS() + 19. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
