Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(6)

Issue 2832813002: Drop SiteBoundCredential in favor of CredentialUserData (Closed)

Created:
2 years, 1 month ago by jdoerrie
Modified:
2 years, 1 month ago
CC:
blink-reviews, chromium-reviews, haraken, mac-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Drop SiteBoundCredential in favor of CredentialUserData This change implements dropping the SiteBoundCredential interface in favor of the CredentialUserData mixin. This change was introduced in the Credential Manager API specification with commit a43865bd8aa9842dbc8d8e688d5668c087677eac (https://github.com/w3c/webappsec-credential-management/commit/a43865bd8aa9842dbc8d8e688d5668c087677eac). Intent to Deprecate and Remove: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/X-5zjSOzLC4 R=vasilii@chromium.org,mkwst@chromium.org,jochen@chromium.org BUG=714615 Review-Url: https://codereview.chromium.org/2832813002 Cr-Commit-Position: refs/heads/master@{#467305} Committed: https://chromium.googlesource.com/chromium/src/+/c95e21cadda8ce43f68dc87d1c65d3ae39ab7850

Patch Set 1 #

Total comments: 11

Patch Set 2 : Nits #

Total comments: 5

Patch Set 3 : HTTPS Rename #

Unified diffs Side-by-side diffs Delta from patch set Stats (+66 lines, -254 lines) Patch
A + third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.https.html View 1 2 3 chunks +10 lines, -5 lines 0 comments Download
A + third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.https-expected.txt View 1 2 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/http/tests/credentialmanager/idl.html View 1 1 chunk +0 lines, -86 lines 0 comments Download
D third_party/WebKit/LayoutTests/http/tests/credentialmanager/idl-expected.txt View 1 1 chunk +0 lines, -50 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/mac/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 3 chunks +6 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/platform/win/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 3 chunks +6 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt View 1 3 chunks +6 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 3 chunks +6 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/BUILD.gn View 2 chunks +2 lines, -2 lines 0 comments Download
A + third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.h View 2 chunks +6 lines, -6 lines 0 comments Download
A + third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.cpp View 1 chunk +2 lines, -3 lines 0 comments Download
A + third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.idl View 1 1 chunk +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/FederatedCredential.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/FederatedCredential.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/FederatedCredential.idl View 1 chunk +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.h View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.cpp View 1 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/modules/credentialmanager/PasswordCredential.idl View 1 chunk +2 lines, -1 line 0 comments Download
D third_party/WebKit/Source/modules/credentialmanager/SiteBoundCredential.h View 1 chunk +0 lines, -29 lines 0 comments Download
D third_party/WebKit/Source/modules/credentialmanager/SiteBoundCredential.cpp View 1 chunk +0 lines, -13 lines 0 comments Download
D third_party/WebKit/Source/modules/credentialmanager/SiteBoundCredential.idl View 1 chunk +0 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/modules/modules_idl_files.gni View 1 1 chunk +1 line, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 31 (13 generated)
jdoerrie
Dear all, please review. This is my first adventure in this part of the code-base, ...
2 years, 1 month ago (2017-04-20 12:58:02 UTC) #3
vasilii
https://codereview.chromium.org/2832813002/diff/1/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt File third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt (right): https://codereview.chromium.org/2832813002/diff/1/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt#newcode4685 third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt:4685: getter name Doesn't it have setters for the same. ...
2 years, 1 month ago (2017-04-20 14:21:31 UTC) #4
vasilii
Also you can email blink-dev with an "Intent to deprecate and remove: `SiteBoundCredential`". See https://www.chromium.org/blink/removing-features
2 years, 1 month ago (2017-04-20 14:24:46 UTC) #5
jdoerrie
Thanks for the comments so far, I will follow up with an email to blink-team ...
2 years, 1 month ago (2017-04-21 08:14:57 UTC) #8
vasilii
https://codereview.chromium.org/2832813002/diff/1/third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.idl File third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.idl (right): https://codereview.chromium.org/2832813002/diff/1/third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.idl#newcode9 third_party/WebKit/Source/modules/credentialmanager/CredentialUserData.idl:9: SecureContext, On 2017/04/21 08:14:57, jdoerrie wrote: > On 2017/04/20 ...
2 years, 1 month ago (2017-04-21 08:42:59 UTC) #9
jochen (gone - plz use gerrit)
this requires an entry on chromestatus.com and an intent to deprecate email (https://www.chromium.org/blink#TOC-Web-Platform-Changes:-Process) I'd also ...
2 years, 1 month ago (2017-04-21 08:58:13 UTC) #10
Mike West
https://codereview.chromium.org/2832813002/diff/1/third_party/WebKit/LayoutTests/http/tests/credentialmanager/idl.html File third_party/WebKit/LayoutTests/http/tests/credentialmanager/idl.html (right): https://codereview.chromium.org/2832813002/diff/1/third_party/WebKit/LayoutTests/http/tests/credentialmanager/idl.html#newcode18 third_party/WebKit/LayoutTests/http/tests/credentialmanager/idl.html:18: USVString iconURL; Either in this patch or a subsequent ...
2 years, 1 month ago (2017-04-24 07:34:08 UTC) #11
jdoerrie
Dear all, please take another look. I updated the referenced bug to be the OWP ...
2 years, 1 month ago (2017-04-24 15:05:45 UTC) #13
vasilii
https://codereview.chromium.org/2832813002/diff/20001/third_party/WebKit/LayoutTests/external/wpt/credential-management/idl-expected.txt File third_party/WebKit/LayoutTests/external/wpt/credential-management/idl-expected.txt (right): https://codereview.chromium.org/2832813002/diff/20001/third_party/WebKit/LayoutTests/external/wpt/credential-management/idl-expected.txt#newcode45 third_party/WebKit/LayoutTests/external/wpt/credential-management/idl-expected.txt:45: PASS FederatedCredential interface: new FederatedCredential({ id: "id", provider: "https://example.com", ...
2 years, 1 month ago (2017-04-24 16:46:26 UTC) #14
Mike West
https://codereview.chromium.org/2832813002/diff/20001/third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.html File third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.html (right): https://codereview.chromium.org/2832813002/diff/20001/third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.html#newcode1 third_party/WebKit/LayoutTests/external/wpt/credential-management/idl.html:1: <!DOCTYPE html> Rename this file to `idl.https.html`; otherwise it's ...
2 years, 1 month ago (2017-04-25 07:39:20 UTC) #15
Mike West
Also, nit: Please link to the intent thread in the CL description. :)
2 years, 1 month ago (2017-04-25 07:39:55 UTC) #16
jdoerrie
Next Round :) I also added the intent thread to the description. https://codereview.chromium.org/2832813002/diff/20001/third_party/WebKit/LayoutTests/external/wpt/credential-management/idl-expected.txt File third_party/WebKit/LayoutTests/external/wpt/credential-management/idl-expected.txt ...
2 years, 1 month ago (2017-04-25 09:43:38 UTC) #20
Mike West
Code change LGTM, but note that you'll still need jochen@, et al. to stamp the ...
2 years, 1 month ago (2017-04-25 11:10:28 UTC) #21
vasilii
lgtm
2 years, 1 month ago (2017-04-25 11:31:13 UTC) #24
jochen (gone - plz use gerrit)
(waiting for the intent thread to be resolved)
2 years, 1 month ago (2017-04-25 15:15:59 UTC) #25
jochen (gone - plz use gerrit)
lgtm
2 years, 1 month ago (2017-04-26 10:07:23 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2832813002/40001
2 years, 1 month ago (2017-04-26 10:58:39 UTC) #28
commit-bot: I haz the power
2 years, 1 month ago (2017-04-26 12:35:14 UTC) #31
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c95e21cadda8ce43f68dc87d1c65...

Powered by Google App Engine
This is Rietveld 408576698