|
|
Chromium Code Reviews|
Created:
4 years, 6 months ago by rohitrao (ping after 24h) Modified:
4 years, 6 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUpdates WKWebViewSecurityUtil to handle deprecated enum values.
BUG=619813
TEST=None
Committed: https://crrev.com/b955cf69c3389d002e9af66b8a4f4ab2b1f49007
Cr-Commit-Position: refs/heads/master@{#399748}
Patch Set 1 #
Total comments: 3
Patch Set 2 : TODO #Messages
Total messages: 18 (8 generated)
rohitrao@chromium.org changed reviewers: + eugenebut@chromium.org, justincohen@google.com
https://codereview.chromium.org/2061103003/diff/1/ios/web/web_state/wk_web_vi... File ios/web/web_state/wk_web_view_security_util.mm (right): https://codereview.chromium.org/2061103003/diff/1/ios/web/web_state/wk_web_vi... ios/web/web_state/wk_web_view_security_util.mm:147: default: Eugene asked whether this should be hidden behind a preprocessor guard. My feeling is that the iOS9 SDK is never going to change to include new enum values, and anything newer will have a default clause, so adding a guard doesn't meaningfully protect us from anything. Thoughts? If we're ok with this approach, I'll file a bug for the TODO.
Description was changed from ========== Updates WKWebViewSecurityUtil to handle deprecated enum values. BUG=619813 TEST=None ========== to ========== Updates WKWebViewSecurityUtil to handle deprecated enum values. BUG=619813 TEST=None ==========
rohitrao@chromium.org changed reviewers: + justincohen@chromium.org - justincohen@google.com
rohitrao@chromium.org changed reviewers: + sdefresne@chromium.org
+sdefresne because he commented on the older CL
lgtm https://codereview.chromium.org/2061103003/diff/1/ios/web/web_state/wk_web_vi... File ios/web/web_state/wk_web_view_security_util.mm (right): https://codereview.chromium.org/2061103003/diff/1/ios/web/web_state/wk_web_vi... ios/web/web_state/wk_web_view_security_util.mm:147: default: On 2016/06/14 15:20:32, rohitrao wrote: > Eugene asked whether this should be hidden behind a preprocessor guard. My > feeling is that the iOS9 SDK is never going to change to include new enum > values, and anything newer will have a default clause, so adding a guard doesn't > meaningfully protect us from anything. Thoughts? > > If we're ok with this approach, I'll file a bug for the TODO. I'm totally fine with having default. And I'm not even sure if we ned a TODO here. Having a comment is fine IMO.
The CQ bit was checked by rohitrao@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from eugenebut@chromium.org Link to the patchset: https://codereview.chromium.org/2061103003/#ps20001 (title: "TODO")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2061103003/20001
Publish drafts ... https://codereview.chromium.org/2061103003/diff/1/ios/web/web_state/wk_web_vi... File ios/web/web_state/wk_web_view_security_util.mm (right): https://codereview.chromium.org/2061103003/diff/1/ios/web/web_state/wk_web_vi... ios/web/web_state/wk_web_view_security_util.mm:147: default: On 2016/06/14 15:35:07, Eugene But wrote: > On 2016/06/14 15:20:32, rohitrao wrote: > > Eugene asked whether this should be hidden behind a preprocessor guard. My > > feeling is that the iOS9 SDK is never going to change to include new enum > > values, and anything newer will have a default clause, so adding a guard > doesn't > > meaningfully protect us from anything. Thoughts? > > > > If we're ok with this approach, I'll file a bug for the TODO. > I'm totally fine with having default. And I'm not even sure if we ned a TODO > here. Having a comment is fine IMO. +1 to eugenebut comment.
lgt,
lgtm
Message was sent while issue was closed.
Description was changed from ========== Updates WKWebViewSecurityUtil to handle deprecated enum values. BUG=619813 TEST=None ========== to ========== Updates WKWebViewSecurityUtil to handle deprecated enum values. BUG=619813 TEST=None ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== Updates WKWebViewSecurityUtil to handle deprecated enum values. BUG=619813 TEST=None ========== to ========== Updates WKWebViewSecurityUtil to handle deprecated enum values. BUG=619813 TEST=None Committed: https://crrev.com/b955cf69c3389d002e9af66b8a4f4ab2b1f49007 Cr-Commit-Position: refs/heads/master@{#399748} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b955cf69c3389d002e9af66b8a4f4ab2b1f49007 Cr-Commit-Position: refs/heads/master@{#399748} |
