Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2340873002:
Creates open_url_util files (Closed)
DescriptionCreates a file that contains a function for openingURL's that uses
UIApplication OpenURL(). The function will use the non deprecated
method if its available (iOS10+)
BUG=622735
Committed: https://crrev.com/712e407b3305a0f79c4f4b8d542309bf3ad62a2d
Cr-Commit-Position: refs/heads/master@{#419336}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Small formatting changes #
Messages
Total messages: 23 (12 generated)
Description was changed from ========== Creates open_url_util files BUG=622735 ========== to ========== Update openURL calls to new iOS10 SDK method openURL: has been deprecated on iOS10 in order to comply with the new SDK we need to update to the new method openURL:options:completionHandler: This method only works on iOS10 so we need to make sure to use the old one on iOS9 and below. BUG=622735 ==========
sczs@chromium.org changed reviewers: + eugenebut@chromium.org, pkl@chromium.org
sczs@chromium.org changed reviewers: + sczs@chromium.org
Created the openURL file upstream. Please help me reviewing it.
Please update CL comments to describe what this CL changes (looks like current comments were taken from downstream CL). https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... File ios/chrome/browser/open_url_util.h (right): https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... ios/chrome/browser/open_url_util.h:12: void OpenURLWithCompletionHandler(NSURL* url, Per comment in downstream CL: s/OpenURLWithCompletionHandler/OpenUrlWithCompletionHandler According to C++ Style Guide we should use Url case for acronyms. We should all all caps URL for Objective-C names only. https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... File ios/chrome/browser/open_url_util.mm (right): https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... ios/chrome/browser/open_url_util.mm:5: #import "open_url_util.h" Please use full path for the imports https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... ios/chrome/browser/open_url_util.mm:18: } else { NIT: You don't need else here, since you have an early return in the previous block. Alternatively you can keep |else| and drop |return|.
Description was changed from ========== Update openURL calls to new iOS10 SDK method openURL: has been deprecated on iOS10 in order to comply with the new SDK we need to update to the new method openURL:options:completionHandler: This method only works on iOS10 so we need to make sure to use the old one on iOS9 and below. BUG=622735 ========== to ========== Creates a file that contains a function for openingURL's that uses UIApplication OpenURL(). The function will use the non deprecated method if its available (iOS10+) BUG=622735 ==========
Patchset #2 (id:20001) has been deleted
H! Implemented the changes. Please give them a quick look! https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... File ios/chrome/browser/open_url_util.h (right): https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... ios/chrome/browser/open_url_util.h:12: void OpenURLWithCompletionHandler(NSURL* url, On 2016/09/14 20:20:07, Eugene But wrote: > Per comment in downstream CL: > > s/OpenURLWithCompletionHandler/OpenUrlWithCompletionHandler > > According to C++ Style Guide we should use Url case for acronyms. We should all > all caps URL for Objective-C names only. Done. https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... File ios/chrome/browser/open_url_util.mm (right): https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... ios/chrome/browser/open_url_util.mm:5: #import "open_url_util.h" On 2016/09/14 20:20:07, Eugene But wrote: > Please use full path for the imports Done. https://codereview.chromium.org/2340873002/diff/1/ios/chrome/browser/open_url... ios/chrome/browser/open_url_util.mm:18: } else { On 2016/09/14 20:20:07, Eugene But wrote: > NIT: You don't need else here, since you have an early return in the previous > block. Alternatively you can keep |else| and drop |return|. Done.
lgtm
The CQ bit was checked by sczs@chromium.org
The CQ bit was unchecked by sczs@chromium.org
lgtm
The CQ bit was checked by sczs@chromium.org
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
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
On 2016/09/16 18:16:15, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, > http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...) Had to create a CL to update upstream bots, will retry patching after this has been done.
The CQ bit was checked by sczs@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Creates a file that contains a function for openingURL's that uses UIApplication OpenURL(). The function will use the non deprecated method if its available (iOS10+) BUG=622735 ========== to ========== Creates a file that contains a function for openingURL's that uses UIApplication OpenURL(). The function will use the non deprecated method if its available (iOS10+) BUG=622735 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Creates a file that contains a function for openingURL's that uses UIApplication OpenURL(). The function will use the non deprecated method if its available (iOS10+) BUG=622735 ========== to ========== Creates a file that contains a function for openingURL's that uses UIApplication OpenURL(). The function will use the non deprecated method if its available (iOS10+) BUG=622735 Committed: https://crrev.com/712e407b3305a0f79c4f4b8d542309bf3ad62a2d Cr-Commit-Position: refs/heads/master@{#419336} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/712e407b3305a0f79c4f4b8d542309bf3ad62a2d Cr-Commit-Position: refs/heads/master@{#419336} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
