Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(677)

Unified Diff: third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp

Issue 2864493003: Deprecate CredentialRequestOptions.unmediated in favor mediation enum (Closed)
Patch Set: Test Comments Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
diff --git a/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp b/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
index ff35965185a0314d8dca14c145930f627e48eadc..8b69934a5aae0e7754d57b57ac8b6bc58881049e 100644
--- a/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
+++ b/third_party/WebKit/Source/modules/credentialmanager/CredentialsContainer.cpp
@@ -15,6 +15,7 @@
#include "core/dom/ExecutionContext.h"
#include "core/frame/Frame.h"
#include "core/frame/UseCounter.h"
+#include "core/inspector/ConsoleMessage.h"
#include "core/page/FrameTree.h"
#include "modules/credentialmanager/Credential.h"
#include "modules/credentialmanager/CredentialManagerClient.h"
@@ -28,6 +29,7 @@
#include "public/platform/WebCredential.h"
#include "public/platform/WebCredentialManagerClient.h"
#include "public/platform/WebCredentialManagerError.h"
+#include "public/platform/WebCredentialMediationRequirement.h"
#include "public/platform/WebFederatedCredential.h"
#include "public/platform/WebPasswordCredential.h"
@@ -168,6 +170,27 @@ ScriptPromise CredentialsContainer::get(
if (!CheckBoilerplate(resolver))
return promise;
+ ExecutionContext* context = ExecutionContext::From(script_state);
+ // Set the default mediation option if none is provided.
+ // If both 'unmediated' and 'mediation' are set log a warning if they are
+ // contradicting.
+ // Also sets 'mediation' appropriately when only 'unmediated' is set.
+ // TODO(http://crbug.com/715077): Remove this when 'unmediated' is removed.
+ String mediation = "optional";
+ if (options.hasUnmediated() && !options.hasMediation()) {
+ mediation = options.unmediated() ? "silent" : "optional";
+ } else if (options.hasMediation()) {
+ mediation = options.mediation();
Mike West 2017/05/19 15:29:56 Please add a test for invalid values.
jdoerrie 2017/05/19 16:22:37 That already exists in credentialscontainer-get-er
+ if (options.hasUnmediated() &&
+ ((options.unmediated() && options.mediation() != "silent") ||
+ (!options.unmediated() && options.mediation() != "optional"))) {
+ context->AddConsoleMessage(ConsoleMessage::Create(
+ kJSMessageSource, kWarningMessageLevel,
+ "mediation: '" + options.mediation() + "' overrides unmediated: " +
+ (options.unmediated() ? "true" : "false") + "."));
Mike West 2017/05/19 15:29:57 Please add a test for this behavior.
jdoerrie 2017/05/19 16:22:37 Done, please see credentialscontainer-get-warning
+ }
+ }
+
Vector<KURL> providers;
if (options.hasFederated() && options.federated().hasProviders()) {
for (const auto& string : options.federated().providers()) {
@@ -177,14 +200,26 @@ ScriptPromise CredentialsContainer::get(
}
}
- UseCounter::Count(ExecutionContext::From(script_state),
- options.unmediated()
- ? UseCounter::kCredentialManagerGetWithoutUI
- : UseCounter::kCredentialManagerGetWithUI);
+ UseCounter::Feature feature;
+ WebCredentialMediationRequirement requirement;
+
+ if (mediation == "silent") {
+ feature = UseCounter::kCredentialManagerGetMediationSilent;
+ requirement = WebCredentialMediationRequirement::kSilent;
+ } else if (mediation == "optional") {
+ feature = UseCounter::kCredentialManagerGetMediationOptional;
+ requirement = WebCredentialMediationRequirement::kOptional;
+ } else {
+ DCHECK_EQ("required", mediation);
+ feature = UseCounter::kCredentialManagerGetMediationRequired;
+ requirement = WebCredentialMediationRequirement::kRequired;
+ }
- CredentialManagerClient::From(ExecutionContext::From(script_state))
- ->DispatchGet(options.unmediated(), options.password(), providers,
- new RequestCallbacks(resolver));
+ UseCounter::Count(context, feature);
Mike West 2017/05/19 15:29:57 Optional nit: I think it's simpler to just call `C
jdoerrie 2017/05/19 16:22:37 Done.
+
+ CredentialManagerClient::From(context)->DispatchGet(
+ requirement, options.password(), providers,
+ new RequestCallbacks(resolver));
return promise;
}

Powered by Google App Engine
This is Rietveld 408576698