[SAA] Remove unnecessary method and fix histogram
OBSOLETE_HISTOGRAMS=API.StorageAccess.RequestStorageAccess is obsolete since the buckets have changed (new grants and existing grants are now indistinguishable in the renderer)
Bug: 1464619
Change-Id: If5747acc48bfc8576effb44b312bce8d967cecce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4727813
Reviewed-by: Shuran Huang <shuuran@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1177305}
diff --git a/third_party/blink/renderer/core/dom/document.cc b/third_party/blink/renderer/core/dom/document.cc
index 1334f4a..2ebdf863 100644
--- a/third_party/blink/renderer/core/dom/document.cc
+++ b/third_party/blink/renderer/core/dom/document.cc
@@ -394,10 +394,11 @@
REJECTED_INSECURE_CONTEXT = 9,
APPROVED_PRIMARY_FRAME = 10,
REJECTED_CREDENTIALLESS_IFRAME = 11,
- kMaxValue = REJECTED_CREDENTIALLESS_IFRAME,
+ APPROVED_NEW_OR_EXISTING_GRANT = 12,
+ kMaxValue = APPROVED_NEW_OR_EXISTING_GRANT,
};
void FireRequestStorageAccessHistogram(RequestStorageResult result) {
- base::UmaHistogramEnumeration("API.StorageAccess.RequestStorageAccess",
+ base::UmaHistogramEnumeration("API.StorageAccess.RequestStorageAccess2",
result);
}
@@ -6515,7 +6516,7 @@
->RequestPermission(
std::move(descriptor),
LocalFrame::HasTransientUserActivation(GetFrame()),
- WTF::BindOnce(&Document::OnRequestedStorageAccessPermissionState,
+ WTF::BindOnce(&Document::ProcessStorageAccessPermissionState,
WrapPersistent(this), WrapPersistent(resolver)));
return promise;
@@ -6560,19 +6561,6 @@
WrapPersistent(this), WrapPersistent(resolver)));
}
-void Document::OnRequestedStorageAccessPermissionState(
- ScriptPromiseResolver* resolver,
- mojom::blink::PermissionStatus status) {
- DCHECK(resolver);
- DCHECK(GetFrame());
- ScriptState* script_state = resolver->GetScriptState();
- DCHECK(script_state);
- ScriptState::Scope scope(script_state);
-
- ProcessStorageAccessPermissionState(resolver,
- /*use_existing_status=*/false, status);
-}
-
void Document::OnRequestedTopLevelStorageAccessPermissionState(
ScriptPromiseResolver* resolver,
mojom::blink::PermissionStatus status) {
@@ -6589,19 +6577,16 @@
void Document::ProcessStorageAccessPermissionState(
ScriptPromiseResolver* resolver,
- bool use_existing_status,
mojom::blink::PermissionStatus status) {
DCHECK(resolver);
DCHECK(GetFrame());
+ ScriptState* script_state = resolver->GetScriptState();
+ DCHECK(script_state);
+ ScriptState::Scope scope(script_state);
if (status == mojom::blink::PermissionStatus::GRANTED) {
- if (use_existing_status) {
- FireRequestStorageAccessHistogram(
- RequestStorageResult::APPROVED_EXISTING_ACCESS);
- } else {
- FireRequestStorageAccessHistogram(
- RequestStorageResult::APPROVED_NEW_GRANT);
- }
+ FireRequestStorageAccessHistogram(
+ RequestStorageResult::APPROVED_NEW_OR_EXISTING_GRANT);
dom_window_->SetHasStorageAccess();
resolver->Resolve();
} else {
@@ -6612,8 +6597,6 @@
mojom::blink::ConsoleMessageSource::kSecurity,
mojom::blink::ConsoleMessageLevel::kError,
"requestStorageAccess: Permission denied."));
- ScriptState* script_state = resolver->GetScriptState();
- DCHECK(script_state);
resolver->Reject(V8ThrowDOMException::CreateOrEmpty(
script_state->GetIsolate(), DOMExceptionCode::kNotAllowedError,
"requestStorageAccess not allowed"));
diff --git a/third_party/blink/renderer/core/dom/document.h b/third_party/blink/renderer/core/dom/document.h
index 88a3b5a..49023694 100644
--- a/third_party/blink/renderer/core/dom/document.h
+++ b/third_party/blink/renderer/core/dom/document.h
@@ -2191,12 +2191,6 @@
mojom::blink::PermissionDescriptorPtr descriptor,
mojom::blink::PermissionStatus previous_status);
- // Wraps `ProcessStorageAccessPermissionState` to handle the requested
- // permission status.
- void OnRequestedStorageAccessPermissionState(
- ScriptPromiseResolver* resolver,
- mojom::blink::PermissionStatus status);
-
// Similar to `OnRequestedStorageAccessPermissionState`, but for the top-level
// variant. Used to react to the result of a permission request.
void OnRequestedTopLevelStorageAccessPermissionState(
@@ -2207,7 +2201,6 @@
// otherwise, and consumes user activation.
void ProcessStorageAccessPermissionState(
ScriptPromiseResolver* resolver,
- bool use_existing_status,
mojom::blink::PermissionStatus status);
// Similar to `ProcessStorageAccessPermissionState`, but for the top-level
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index 3bcb5ffd..3344cba 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -92816,6 +92816,7 @@
<int value="9" label="Rejected due to insecure context"/>
<int value="10" label="Approved due to being called from top-level frame"/>
<int value="11" label="Rejected due to credentialless iframe"/>
+ <int value="12" label="Approved with a new or existing grant"/>
</enum>
<enum name="ResolutionCategory">
diff --git a/tools/metrics/histograms/metadata/storage/histograms.xml b/tools/metrics/histograms/metadata/storage/histograms.xml
index c51f22a..c062ac24 100644
--- a/tools/metrics/histograms/metadata/storage/histograms.xml
+++ b/tools/metrics/histograms/metadata/storage/histograms.xml
@@ -121,16 +121,17 @@
</summary>
</histogram>
-<histogram name="API.StorageAccess.RequestStorageAccess"
+<histogram name="API.StorageAccess.RequestStorageAccess2"
enum="RequestStorageResult" expires_after="2023-12-24">
<owner>mkwst@chromium.org</owner>
+ <owner>cfredric@chromium.org</owner>
<owner>brandm@microsoft.com</owner>
<summary>
Records requests to use document.requestStorageAccess and reasons the
request may be approved or rejected.
- Warning: this histogram was expired from 2020-11-27 to 08-12-2022; data may
- be missing.
+ Recorded by the renderer exactly once for each invocation of
+ document.requestStorageAccess.
</summary>
</histogram>
