[WV] Determine if partitioned cookies are not sent to apps.
This allows an investigation into how often partitioned cookies are not passed to apps. Using CookieManager::SiteHasCookieInOtherPartition, this indicates whether a site's cookies, with one partition key, also has the same cookies with a different cookie partition key. This will prove that there are cookies being omitted on a given site due to the partitioning. Co-Authors: bewise, avvall Bug: 41496912 Change-Id: I3698788038afc16c23bb05f6e7f485394433fd07 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6237015 Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Elly FJ <ellyjones@chromium.org> Reviewed-by: Maks Orlovich <morlovich@chromium.org> Commit-Queue: Adam Walls <avvall@chromium.org> Cr-Commit-Position: refs/heads/main@{#1417004}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
3d0d9a7458
commit
cbebd148e6
android_webview
browser
common
java
src
org
chromium
android_webview
javatests
src
org
chromium
android_webview
services/network
tools/metrics/histograms/metadata/android
@ -56,6 +56,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
|
||||
&sensitive_content::features::kSensitiveContent,
|
||||
&features::kWebViewWebauthn,
|
||||
&::features::kPrefetchBrowserInitiatedTriggers,
|
||||
&features::kWebViewPartitionedCookiesExcluded,
|
||||
};
|
||||
|
||||
// static
|
||||
|
@ -20,6 +20,7 @@
|
||||
#include "android_webview/browser/aw_browser_context_store.h"
|
||||
#include "android_webview/browser/aw_client_hints_controller_delegate.h"
|
||||
#include "android_webview/browser/aw_cookie_access_policy.h"
|
||||
#include "android_webview/common/aw_features.h"
|
||||
#include "android_webview/common/aw_switches.h"
|
||||
#include "base/android/build_info.h"
|
||||
#include "base/android/callback_android.h"
|
||||
@ -28,6 +29,7 @@
|
||||
#include "base/android/scoped_java_ref.h"
|
||||
#include "base/command_line.h"
|
||||
#include "base/containers/circular_deque.h"
|
||||
#include "base/feature_list.h"
|
||||
#include "base/files/file_util.h"
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/functional/callback_helpers.h"
|
||||
@ -94,6 +96,8 @@ void MaybeRunCookieCallback(base::OnceCallback<void(bool)> callback,
|
||||
}
|
||||
|
||||
const char kSecureCookieHistogramName[] = "Android.WebView.SecureCookieAction";
|
||||
const char kPartitionedCookiesAreExcludedHistogramName[] =
|
||||
"Android.WebView.PartitionedCookiesExcluded";
|
||||
|
||||
// These values are persisted to logs. Entries should not be renumbered and
|
||||
// numeric values should never be reused.
|
||||
@ -587,6 +591,19 @@ void CookieManager::GetCookieListAsyncHelper(const GURL& host,
|
||||
base::BindOnce(&CookieManager::GetCookieListCompleted,
|
||||
base::Unretained(this), std::move(complete), result));
|
||||
}
|
||||
|
||||
if (base::FeatureList::IsEnabled(
|
||||
features::kWebViewPartitionedCookiesExcluded)) {
|
||||
// We only want to execute this for metrics so we offload this work to later
|
||||
// so that we don't block the sync get cookies operation on this call.
|
||||
// This won't be looking at the same state as the initial GetCookie call
|
||||
// but we are okay with that because Android developers are likely going
|
||||
// to try call this after the cookie they care about has been set.
|
||||
cookie_store_task_runner_->PostTask(
|
||||
FROM_HERE,
|
||||
base::BindOnce(&CookieManager::RecordExcludedPartitionedCookies,
|
||||
base::Unretained(this), host));
|
||||
}
|
||||
}
|
||||
|
||||
void CookieManager::GetCookieListCompleted(
|
||||
@ -795,6 +812,32 @@ void CookieManager::ClearClientHintsCachedPerOriginMapIfNeeded() {
|
||||
}
|
||||
}
|
||||
|
||||
void CookieManager::RecordExcludedPartitionedCookies(const GURL& host) {
|
||||
auto record_cookies_excluded_callback =
|
||||
base::BindOnce([](std::optional<bool> are_excluded) {
|
||||
if (are_excluded.has_value()) {
|
||||
base::UmaHistogramBoolean(kPartitionedCookiesAreExcludedHistogramName,
|
||||
*are_excluded);
|
||||
}
|
||||
});
|
||||
|
||||
net::CookiePartitionKey cpk = net::CookiePartitionKey::FromWire(
|
||||
net::SchemefulSite(host),
|
||||
net::CookiePartitionKey::AncestorChainBit::kSameSite);
|
||||
|
||||
if (GetMojoCookieManager()) {
|
||||
GetMojoCookieManager()->SiteHasCookieInOtherPartition(
|
||||
net::SchemefulSite(host),
|
||||
std::optional<net::CookiePartitionKey>(net::CookiePartitionKey(cpk)),
|
||||
std::move(record_cookies_excluded_callback));
|
||||
} else {
|
||||
std::optional<bool> cookies_excluded =
|
||||
GetCookieStore()->SiteHasCookieInOtherPartition(
|
||||
net::SchemefulSite(host), cpk);
|
||||
std::move(record_cookies_excluded_callback).Run(cookies_excluded);
|
||||
}
|
||||
}
|
||||
|
||||
static jlong JNI_AwCookieManager_GetDefaultCookieManager(JNIEnv* env) {
|
||||
return reinterpret_cast<intptr_t>(CookieManager::GetDefaultInstance());
|
||||
}
|
||||
|
@ -258,6 +258,10 @@ class CookieManager {
|
||||
// need to clear them later.
|
||||
void ClearClientHintsCachedPerOriginMapIfNeeded();
|
||||
|
||||
// We want to detect if there are any apps requesting cookies that may not
|
||||
// get all the cookies for a particular URL back due to CHIPS being enabled.
|
||||
void RecordExcludedPartitionedCookies(const GURL& host);
|
||||
|
||||
// Returns the AwBrowserContext associated with the same profile as this
|
||||
// CookieManager. For the default profile, the AwBrowserContext is not
|
||||
// guaranteed to be initialized, so it may be null.
|
||||
|
@ -89,6 +89,13 @@ BASE_FEATURE(kWebViewMuteAudio,
|
||||
"WebViewMuteAudio",
|
||||
base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
|
||||
// When enabled, WebView records if a site with partitioned cookies has any
|
||||
// cookies excluded due to a different cookie partition key than the current
|
||||
// site's.
|
||||
BASE_FEATURE(kWebViewPartitionedCookiesExcluded,
|
||||
"WebViewPartitionedCookiesExcluded",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT);
|
||||
|
||||
// Only allow extra headers added via loadUrl() to be sent to the original
|
||||
// origin; strip them from the request if a cross-origin redirect occurs.
|
||||
BASE_FEATURE(kWebViewExtraHeadersSameOriginOnly,
|
||||
|
@ -30,6 +30,7 @@ BASE_DECLARE_FEATURE(kWebViewLazyFetchHandWritingIcon);
|
||||
BASE_DECLARE_FEATURE(kWebViewMediaIntegrityApiBlinkExtension);
|
||||
BASE_DECLARE_FEATURE(kWebViewMixedContentAutoupgrades);
|
||||
BASE_DECLARE_FEATURE(kWebViewMuteAudio);
|
||||
BASE_DECLARE_FEATURE(kWebViewPartitionedCookiesExcluded);
|
||||
BASE_DECLARE_FEATURE(kWebViewRecordAppDataDirectorySize);
|
||||
BASE_DECLARE_FEATURE(kWebViewRenderDocument);
|
||||
BASE_DECLARE_FEATURE(kWebViewRestrictSensitiveContent);
|
||||
|
@ -1042,6 +1042,10 @@ public final class ProductionSupportedFlagList {
|
||||
Flag.baseFeature(
|
||||
MediaFeatures.MEDIA_CODEC_BLOCK_MODEL,
|
||||
"Controls use of MediaCodec's LinearBlock mode."),
|
||||
Flag.baseFeature(
|
||||
AwFeatures.WEBVIEW_PARTITIONED_COOKIES_EXCLUDED,
|
||||
"When enabled, WebView records if a site with partitioned cookies has any cookies"
|
||||
+ " excluded due to a different cookie partition key than the current site's."),
|
||||
// Add new commandline switches and features above. The final entry should have a
|
||||
// trailing comma for cleaner diffs.
|
||||
};
|
||||
|
@ -94,6 +94,8 @@ public class CookieManagerTest extends AwParameterizedTest {
|
||||
private AwContents mAwContents;
|
||||
|
||||
private static final String SECURE_COOKIE_HISTOGRAM_NAME = "Android.WebView.SecureCookieAction";
|
||||
private static final String PARTITIONED_COOKIES_EXCLUDED_HISTOGRAM_NAME =
|
||||
"Android.WebView.PartitionedCookiesExcluded";
|
||||
|
||||
public CookieManagerTest(AwSettingsMutation param) {
|
||||
this.mActivityTestRule = new AwActivityTestRule(param.getMutation());
|
||||
@ -1265,6 +1267,7 @@ public class CookieManagerTest extends AwParameterizedTest {
|
||||
@MediumTest
|
||||
@Feature({"AndroidWebView", "Privacy"})
|
||||
@CommandLineFlags.Add("enable-features=WebViewInterceptedCookieHeader")
|
||||
@Features.EnableFeatures({AwFeatures.WEBVIEW_PARTITIONED_COOKIES_EXCLUDED})
|
||||
public void testPartitionedNetCookies() throws Throwable {
|
||||
TestAwContentsClient.ShouldInterceptRequestHelper shouldInterceptRequestHelper =
|
||||
mContentsClient.getShouldInterceptRequestHelper();
|
||||
@ -1314,6 +1317,16 @@ public class CookieManagerTest extends AwParameterizedTest {
|
||||
expectedCookies,
|
||||
webServer.getLastRequest("/path_to_intercept").headerValue("Cookie"));
|
||||
|
||||
// The cookie manager will only return top level partitioned cookies.
|
||||
// We want to measure that the app will not get all cookies back.
|
||||
try (var histogramWatcher =
|
||||
HistogramWatcher.newBuilder()
|
||||
.expectBooleanRecord(PARTITIONED_COOKIES_EXCLUDED_HISTOGRAM_NAME, true)
|
||||
.build()) {
|
||||
mCookieManager.getCookieInfo(iframeUrl);
|
||||
histogramWatcher.pollInstrumentationThreadUntilSatisfied();
|
||||
}
|
||||
|
||||
// TODO(crbug.com/384986095): Re-add the real expected cookie behavior
|
||||
// post-experimentation
|
||||
String interceptRequestFailureMessage =
|
||||
@ -1369,6 +1382,7 @@ public class CookieManagerTest extends AwParameterizedTest {
|
||||
@MediumTest
|
||||
@Feature({"AndroidWebView", "Privacy"})
|
||||
@CommandLineFlags.Add("disable-partitioned-cookies")
|
||||
@Features.EnableFeatures({AwFeatures.WEBVIEW_PARTITIONED_COOKIES_EXCLUDED})
|
||||
public void testDisabledPartitionedNetCookies() throws Throwable {
|
||||
TestWebServer webServer = TestWebServer.startSsl();
|
||||
|
||||
@ -1412,6 +1426,16 @@ public class CookieManagerTest extends AwParameterizedTest {
|
||||
"partitioned_cookie=foo; unpartitioned_cookie=bar",
|
||||
webServer.getLastRequest("/path_to_intercept").headerValue("Cookie"));
|
||||
|
||||
// The cookie manager will only return top level partitioned cookies.
|
||||
// We want to measure that if CHIPS isn't enabled, all cookies should be returned.
|
||||
try (var histogramWatcher =
|
||||
HistogramWatcher.newBuilder()
|
||||
.expectBooleanRecord(PARTITIONED_COOKIES_EXCLUDED_HISTOGRAM_NAME, false)
|
||||
.build()) {
|
||||
mCookieManager.getCookieInfo(iframeUrl);
|
||||
histogramWatcher.pollInstrumentationThreadUntilSatisfied();
|
||||
}
|
||||
|
||||
blockThirdPartyCookies(mAwContents);
|
||||
mActivityTestRule.loadUrlSync(
|
||||
mAwContents, mContentsClient.getOnPageFinishedHelper(), url);
|
||||
|
@ -175,6 +175,14 @@ void CookieManager::DeleteCanonicalCookie(
|
||||
}).Then(std::move(callback)));
|
||||
}
|
||||
|
||||
void CookieManager::SiteHasCookieInOtherPartition(
|
||||
const net::SchemefulSite& schemeful_site,
|
||||
const std::optional<net::CookiePartitionKey>& cookie_partition_key,
|
||||
SiteHasCookieInOtherPartitionCallback callback) {
|
||||
std::move(callback).Run(cookie_store_->SiteHasCookieInOtherPartition(
|
||||
schemeful_site, cookie_partition_key));
|
||||
}
|
||||
|
||||
void CookieManager::SetContentSettings(
|
||||
ContentSettingsType content_settings_type,
|
||||
const ContentSettingsForOneType& settings,
|
||||
|
@ -87,6 +87,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CookieManager
|
||||
SetCanonicalCookieCallback callback) override;
|
||||
void DeleteCanonicalCookie(const net::CanonicalCookie& cookie,
|
||||
DeleteCanonicalCookieCallback callback) override;
|
||||
void SiteHasCookieInOtherPartition(
|
||||
const net::SchemefulSite& schemeful_site,
|
||||
const std::optional<net::CookiePartitionKey>& cookie_partition_key,
|
||||
SiteHasCookieInOtherPartitionCallback callback) override;
|
||||
void SetContentSettings(ContentSettingsType content_settings_type,
|
||||
const ContentSettingsForOneType& settings,
|
||||
SetContentSettingsCallback callback) override;
|
||||
|
@ -437,6 +437,17 @@ interface CookieManager {
|
||||
// Delete a cookie. Returns true if a cookie was deleted.
|
||||
DeleteCanonicalCookie(CanonicalCookie cookie) => (bool success);
|
||||
|
||||
// Returns true if a site has cookies in a separate partition than the
|
||||
// current site's partition.
|
||||
// If a nullopt is returned, it means either the partition key wasn't set
|
||||
// or the cookie store isn't fully initialized yet.
|
||||
// Note: Calls here do not follow normal sequencing rules. The results
|
||||
// are processed and returned to the caller as soon as requests
|
||||
// are received.
|
||||
SiteHasCookieInOtherPartition(SchemefulSite schemeful_site,
|
||||
CookiePartitionKey? partition_key)
|
||||
=> (bool? site_has_cookie_in_other_partition);
|
||||
|
||||
// Delete a set of cookies matching the passed filter.
|
||||
// Returns the number of cookies deleted.
|
||||
DeleteCookies(CookieDeletionFilter filter) => (uint32 num_deleted);
|
||||
|
@ -66,6 +66,10 @@ class TestCookieManager : public network::mojom::CookieManager {
|
||||
void SetMitigationsEnabledFor3pcd(bool enable) override {}
|
||||
void SetTrackingProtectionEnabledFor3pcd(bool enable) override {}
|
||||
void SetPreCommitCallbackDelayForTesting(base::TimeDelta delay) override {}
|
||||
void SiteHasCookieInOtherPartition(
|
||||
const net::SchemefulSite& schemeful_site,
|
||||
const std::optional<net::CookiePartitionKey>& cookie_partition_key,
|
||||
SiteHasCookieInOtherPartitionCallback callback) override {}
|
||||
|
||||
virtual void DispatchCookieChange(const net::CookieChangeInfo& change);
|
||||
|
||||
|
@ -7065,6 +7065,21 @@ chromium-metrics-reviews@google.com.
|
||||
</summary>
|
||||
</histogram>
|
||||
|
||||
<histogram name="Android.WebView.PartitionedCookiesExcluded"
|
||||
enum="BooleanEnabled" expires_after="2025-08-01">
|
||||
<owner>avvall@chromium.org</owner>
|
||||
<owner>bewise@chromium.org</owner>
|
||||
<owner>src/android_webview/OWNERS</owner>
|
||||
<summary>
|
||||
Records whether or not partitioned cookies are omitted from the current
|
||||
site. This means that there are cookies in the cookie jar with separate
|
||||
partition keys.
|
||||
|
||||
This is recorded when Android developers get cookies using the CookieManager
|
||||
API if the WebViewPartitionedCookiesExcluded flag is enabled.
|
||||
</summary>
|
||||
</histogram>
|
||||
|
||||
<histogram name="Android.WebView.PrimaryCpuAbiBitness"
|
||||
enum="PrimaryCpuAbiBitness" expires_after="2025-04-30">
|
||||
<owner>tasak@google.com</owner>
|
||||
|
Reference in New Issue
Block a user