New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774736 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug

Blocking:
issue 768876



Sign in to add a comment

Remove use of -openURL: from external_app_launcher.mm

Project Member Reported by pkl@chromium.org, Oct 14 2017

Issue description

UIApplication -openURL: has been deprecated. Use -openURL:options:completionHandler: instead.

external_app_launcher.mm has the last reference to this call.

Since -openURL:options:completionHandler: is asynchronous, all calls must also be converted to be asynchronous. Currently, the call in external_app_launcher uses the return value from -openURL to return in the outer call.
 

Comment 1 by pkl@chromium.org, Oct 14 2017

Blocking: 768876

Comment 2 by pkl@chromium.org, Oct 14 2017

Status: Started (was: Assigned)

Comment 3 by pkl@chromium.org, Oct 14 2017

Labels: M-64

Comment 4 by pkl@chromium.org, Nov 16 2017

Cc: eugene...@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 16 2017

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

commit 445feb54df5c9158e015bc727a17ebc36d38ebba
Author: Peter K. Lee <pkl@chromium.org>
Date: Thu Nov 16 18:17:09 2017

Calls decisionsHandler asynchronously

This change is a precursor to a more extensive change where
ExternalAppLauncher API is changed to be asynchronous.

This simpler change is to call the decision handler in the implemention
of WKNavigateDelegate method
webView:decidePolicyForNavigationResponse:decisionHandler:
asynchronously on iOS 11 and up.

iOS 10 (and possible below) has a bug where JavaScript on the page would
not execute if the decision handler is called asynchronously.

This change is to make sure that there are no other unforeseen
issues with calling decision handler asynchronously on iOS 11.

Bug: 774736
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7a6dfe637cb6be4ce5c7efe5824728b8f06b902a
Reviewed-on: https://chromium-review.googlesource.com/773570
Commit-Queue: Peter Lee <pkl@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517129}
[modify] https://crrev.com/445feb54df5c9158e015bc727a17ebc36d38ebba/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 19 2017

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

commit 2d1368a2277f3b36bbe2c5b5a1bf1ed8a55b2501
Author: Peter K. Lee <pkl@chromium.org>
Date: Sun Nov 19 06:11:02 2017

Calls decisionHandler asynchronously, part 2

Only one of two calls to decisionHandler was converted to async in
http://crrev/c/773570. This changes the other call.

Bug: 774736
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ie72c6efc42e5921eae8983d01f4be29fd3934d98
Reviewed-on: https://chromium-review.googlesource.com/777400
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517730}
[modify] https://crrev.com/2d1368a2277f3b36bbe2c5b5a1bf1ed8a55b2501/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 21 2017

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

commit dd94f65429e48697a50269b65e7b2e0972414bfa
Author: Peter Lee <pkl@chromium.org>
Date: Tue Nov 21 19:54:26 2017

Revert "Calls decisionsHandler asynchronously"

This reverts commit 445feb54df5c9158e015bc727a17ebc36d38ebba.

Reason for revert: This will break on iOS 11.2 beta.

Original change's description:
> Calls decisionsHandler asynchronously
>
> This change is a precursor to a more extensive change where
> ExternalAppLauncher API is changed to be asynchronous.
>
> This simpler change is to call the decision handler in the implemention
> of WKNavigateDelegate method
> webView:decidePolicyForNavigationResponse:decisionHandler:
> asynchronously on iOS 11 and up.
>
> iOS 10 (and possible below) has a bug where JavaScript on the page would
> not execute if the decision handler is called asynchronously.
>
> This change is to make sure that there are no other unforeseen
> issues with calling decision handler asynchronously on iOS 11.
>
> Bug: 774736
> Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
> Change-Id: I7a6dfe637cb6be4ce5c7efe5824728b8f06b902a
> Reviewed-on: https://chromium-review.googlesource.com/773570
> Commit-Queue: Peter Lee <pkl@chromium.org>
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#517129}

TBR=eugenebut@chromium.org,pkl@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 774736
Change-Id: I007149ee9ef15d4187a76d66f90bc52d21ff09aa
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/782682
Commit-Queue: Peter Lee <pkl@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518342}
[modify] https://crrev.com/dd94f65429e48697a50269b65e7b2e0972414bfa/ios/web/web_state/ui/crw_web_controller.mm

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 21 2017

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

commit 6d763d5a9185e477b9c0b9c87c03c9480a558101
Author: Peter Lee <pkl@chromium.org>
Date: Tue Nov 21 20:33:41 2017

Revert "Calls decisionHandler asynchronously, part 2"

This reverts commit 2d1368a2277f3b36bbe2c5b5a1bf1ed8a55b2501.

Reason for revert: This will break on iOS 11.2 beta 

Original change's description:
> Calls decisionHandler asynchronously, part 2
> 
> Only one of two calls to decisionHandler was converted to async in
> http://crrev/c/773570. This changes the other call.
> 
> Bug: 774736
> Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
> Change-Id: Ie72c6efc42e5921eae8983d01f4be29fd3934d98
> Reviewed-on: https://chromium-review.googlesource.com/777400
> Reviewed-by: Eugene But <eugenebut@chromium.org>
> Commit-Queue: Peter Lee <pkl@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#517730}

TBR=eugenebut@chromium.org,pkl@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 774736
Change-Id: I589fdeee11059b25edc1f755c0411f8ff9d7e87f
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/782744
Reviewed-by: Peter Lee <pkl@chromium.org>
Commit-Queue: Peter Lee <pkl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518371}
[modify] https://crrev.com/6d763d5a9185e477b9c0b9c87c03c9480a558101/ios/web/web_state/ui/crw_web_controller.mm

Comment 9 by pkl@chromium.org, Dec 4 2017

Labels: -M-64 M-65
What's the status on this please? Is it still blocking iOS 9 deprecation?

Comment 11 by pkl@chromium.org, Dec 5 2017

No, this is no longer blocking iOS 9 deprecation. 

The deprecated methods warnings are being suppressed by this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/787471/3/ios/chrome/browser/web/external_app_launcher.mm
Description: Show this description

Comment 13 by pkl@chromium.org, Apr 12 2018

Cc: pkl@chromium.org
Labels: -Pri-2 -M-65 Pri-3
Owner: ----
Status: Available (was: Started)
No time to work on this, but we should stop using the deprecated API. With new iOS releases, the API may actually be "upgraded" from being deprecated to disappearing.

mrefaat will work on the External App Launcher code and this is something that can be addressed at the same time.
Owner: mrefaat@chromium.org
Status: Assigned (was: Available)
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 2

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

commit 4539b034af60875597b46eaa4b9a8727c55aa670
Author: mrefaat <mrefaat@chromium.org>
Date: Thu Aug 02 22:25:45 2018

Use Asynchronous openURL to launch external apps.

Break dependency between post app launching logic and the app launching
success status, and then use Asynchronous openURL instead to launch apps.

Bug: 774736, 850760
Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Id51a246df9f8a7bd809025d66d987520c4001c6e
Reviewed-on: https://chromium-review.googlesource.com/1152545
Commit-Queue: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580353}
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/BUILD.gn
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/about_flags.mm
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/app_launcher/BUILD.gn
[add] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/app_launcher/app_launcher_flags.h
[add] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/app_launcher/app_launcher_flags.mm
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/app_launcher/app_launcher_tab_helper.mm
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/app_launcher/app_launcher_tab_helper_delegate.h
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/app_launcher/app_launcher_tab_helper_unittest.mm
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/ios_chrome_flag_descriptions.cc
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/ios_chrome_flag_descriptions.h
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/ui/app_launcher/BUILD.gn
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/ui/app_launcher/app_launcher_coordinator.mm
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/chrome/browser/ui/app_launcher/app_launcher_coordinator_unittest.mm
[modify] https://crrev.com/4539b034af60875597b46eaa4b9a8727c55aa670/ios/web/web_state/ui/crw_web_controller.mm

Sign in to add a comment