New issue
Advanced search Search tips

Issue 661445 link

Starred by 1 user
Status: Fixed
Owner:
eugene...@chromium.org
Closed: Jan 2017
Cc:
michaeldo@chromium.org
ios-bugs@chromium.org
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Feature

Blocking:
issue 675001



Sign in to add a comment

Remove old JS Dialogs API from CRWWebUserInterfaceDelegate

Project Member Reported by eugene...@chromium.org, Nov 2 2016 
Remove unused API for presenting alert/confirm/submit dialogs.

Comment 1 by eugene...@chromium.org, Dec 16 2016

Blocking: 675001

Comment 2 by eugene...@chromium.org, Jan 3 2017

Cc: michaeldo@chromium.org
Owner: eugene...@chromium.org
Status: Started (was: Assigned)
I will take this one, unless you already started work.

Comment 3 by michaeldo@chromium.org, Jan 4 2017 

It looks like I already have this change on a branch. If you haven't already started too, I can send a CL.

Comment 4 by michaeldo@chromium.org, Jan 4 2017 

Sorry, it looks like my branch is not complete. Feel free to take this. :)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 6 2017 

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

commit e3cb6842bb7d440a8f8259bea661e5b97159fdca
Author: eugenebut <eugenebut@chromium.org>
Date: Fri Jan 06 18:59:56 2017

[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}

[modify] https://crrev.com/e3cb6842bb7d440a8f8259bea661e5b97159fdca/ios/web/web_state/ui/crw_web_controller_unittest.mm
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 6 2017 

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

commit 62430eb9fc26862ecc4f8b7adb8ad558f4dfc73f
Author: eugenebut <eugenebut@chromium.org>
Date: Fri Jan 06 19:46:01 2017

[ios] Removed deprecated methods from CRWWebUserInterfaceDelegate.

These methods are not implemented by Tab anymore.

BUG= 661445 

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

[modify] https://crrev.com/62430eb9fc26862ecc4f8b7adb8ad558f4dfc73f/ios/web/public/web_state/crw_web_user_interface_delegate.h
[modify] https://crrev.com/62430eb9fc26862ecc4f8b7adb8ad558f4dfc73f/ios/web/web_state/ui/crw_web_controller.mm

Comment 8 by eugene...@chromium.org, Jan 6 2017

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Jan 6 2017 

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

commit 8c9857238e62da4de34c8b02458418916c8a77d0
Author: rohitrao <rohitrao@chromium.org>
Date: Fri Jan 06 20:41:37 2017

Revert of [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. (patchset #1 id:1 of https://codereview.chromium.org/2613163002/ )

Reason for revert:
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

Original issue's 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

TBR=michaeldo@chromium.org,eugenebut@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 661445 

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

[modify] https://crrev.com/8c9857238e62da4de34c8b02458418916c8a77d0/ios/web/web_state/ui/crw_web_controller_unittest.mm

Comment 10 by eugene...@chromium.org, Jan 6 2017

Status: Started (was: Fixed)
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 6 2017 

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

commit 1cec8405fc3e76fb62641082f60787f4af6daad6
Author: rohitrao <rohitrao@chromium.org>
Date: Fri Jan 06 21:44:13 2017

Reland of [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. (patchset #1 id:1 of https://codereview.chromium.org/2615283002/ )

Reason for revert:
Reverting broke tests on the main waterfall, so unreverting =)

Original issue's description:
> Revert of [ios] Test new dialogs API in CRWWebControllerPageDialogOpenPolicyTest. (patchset #1 id:1 of https://codereview.chromium.org/2613163002/ )
>
> Reason for revert:
> 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
>
> Original issue's 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
>
> TBR=michaeldo@chromium.org,eugenebut@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 661445 
>
> Review-Url: https://codereview.chromium.org/2615283002
> Cr-Commit-Position: refs/heads/master@{#442036}
> Committed: https://chromium.googlesource.com/chromium/src/+/8c9857238e62da4de34c8b02458418916c8a77d0

TBR=michaeldo@chromium.org,eugenebut@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 661445 

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

[modify] https://crrev.com/1cec8405fc3e76fb62641082f60787f4af6daad6/ios/web/web_state/ui/crw_web_controller_unittest.mm

Comment 12 by eugene...@chromium.org, Jan 13 2017

Status: Fixed (was: Started)
►

Sign in to add a comment