NavDestination.fillInLabel doesn't take into account NavTypes [388100796]

Fixed

Bug

Status Update

No update yet.

Description

il...@google.com

created issue #1

Jan 6, 2025 11:50PM

Component used: Navigation

Version used: 2.8.5

The NavDestination.fillInLabel method attempts to fill in the label of the NavDestination with arguments. However, it does this by (in every case but NavType.ReferenceType) by just calling args[argName].toString() - e.g., just directly calling toString() on the object in the SavedState bundle.

While this approach works for simple types, a custom NavType might not store their class directly in the Bundle - for example, if using the @Serializable support, it would be stored as a Bundle, which means the toString() is unlikely to actually be what is needed.

Instead, fillInLabel should take into account the NavType and use its get method to give the NavType control over exactly the object that is synthesized from the arguments. That way, developers could implement a custom toString() implementation on their object to control how it is displayed in the label.

Comments

cl...@google.com <cl...@google.com> Jan 7, 2025 12:10AM

Accepted by cl...@google.com.

ap...@google.com <ap...@google.com> #2Jan 7, 2025 04:23PM

Project: chromium/src
Branch: main

commit 6dc78fbbbbadac9c2dc2065150a7af2422c6115c
Author: Rupert Ben Wiser <bewise@chromium.org>
Date: Thu Sep 12 16:16:14 2024

webview: Add support configuration for CHIPS

Adding a boundary interface to let Android developers configure
if partitioned cookies should be enabled or not.

go/chips-android-apis-webview

Bug: 364904765
Change-Id: I683ab5ec157a11aa081db5763380e692ab02c5e6
Reviewed-on:

https://chromium-review.googlesource.com/c/chromium/src/+/5850034
Reviewed-by: Peter Pakkenberg <pbirk@chromium.org>
Auto-Submit: Rupert Wiser <bewise@chromium.org>
Commit-Queue: Peter Pakkenberg <pbirk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1354607}

M android_webview/expectations/system_webview_32_64_bundle.AndroidManifest.expected
M android_webview/expectations/system_webview_32_bundle.AndroidManifest.expected
M android_webview/expectations/system_webview_64_bundle.AndroidManifest.expected
M android_webview/expectations/system_webview_bundle.AndroidManifest.expected
M android_webview/expectations/trichrome_webview_32_64_bundle.AndroidManifest.expected
M android_webview/expectations/trichrome_webview_32_bundle.AndroidManifest.expected
M android_webview/expectations/trichrome_webview_64_32_bundle.AndroidManifest.expected
M android_webview/expectations/trichrome_webview_64_bundle.AndroidManifest.expected
M android_webview/expectations/trichrome_webview_bundle.AndroidManifest.expected
M android_webview/glue/java/src/com/android/webview/chromium/AndroidXProcessGlobalConfig.java
M android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
M android_webview/nonembedded/java/AndroidManifest.xml
M android_webview/support_library/boundary_interfaces/src/org/chromium/support_lib_boundary/ProcessGlobalConfigConstants.java
M chrome/android/expectations/monochrome_32_64_public_bundle__base.AndroidManifest.expected
M chrome/android/expectations/monochrome_32_public_bundle__base.AndroidManifest.expected
M chrome/android/expectations/monochrome_64_32_public_bundle__base.AndroidManifest.expected
M chrome/android/expectations/monochrome_64_public_bundle__base.AndroidManifest.expected
M chrome/android/expectations/monochrome_public_bundle__base.AndroidManifest.expected

https://chromium-review.googlesource.com/5850034

cl...@google.com <cl...@google.com> #3Jan 7, 2025 06:55PM

Marked as fixed.

There is already test coverage of CHIPS behavior inside of Chromium in CookieManagerTest#testPartitionedNetCookies and CookieManagerTest#testDisabledPartitionedNetCookies.

Due to limitations of AndroidX test infra, manual test instructions are necessary to exercise the toggling of the enable partitioned cookies API. WebView toggles CHIPS behavior with the disable-partitioned-cookies command line switch. To demonstrate the API's correctness in toggling this, log statements were inspected when this flag was toggled during WebView startup.

See the diff below for details:

diff --git a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
index 63c5f5a5b2795..ee589176bdab2 100644
--- a/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
+++ b/android_webview/glue/java/src/com/android/webview/chromium/WebViewChromiumFactoryProvider.java
@@ -490,6 +490,9 @@ public class WebViewChromiumFactoryProvider implements WebViewFactoryProvider {
 
             if (!androidXConfig.getPartitionedCookiesEnabled()) {
                 cl.appendSwitch("disable-partitioned-cookies");
+                Log.e(TAG, "Partitioned cookies disabled");
+            } else {
+                Log.e(TAG, "Partitioned cookies enabled");
             }
 
             // Now safe to use WebView data directory.

na...@google.com <na...@google.com> #4Jan 29, 2025 06:11PM

Note: Will add testing when more of the API has been implemented.

zy...@gmail.com <zy...@gmail.com> #5Feb 22, 2025 05:16AM

Project: platform/frameworks/support
Branch: androidx-main
Author: Adam Walls <avvall@google.com>
Link: https://android-review.googlesource.com/3318854

[WebView] Add API to configure CHIPS

Expand for full commit details

[WebView] Add API to configure CHIPS 
 
This adds an API to allow Android developers to configure CHIPS globally. This is done with ProcessGlobalConfig. In order to prevent this from being published/used prior to Android B's release, RestrictTo annotation is used. 
 
Design: go/chips-android-apis-webview 
 
Test: Will add testing in a follow-up CL. 
Bug: 364904765 
Change-Id: If6d630562be7209eb2ab00da2dbad9398ba0553e

Files:

M webkit/webkit/src/main/java/androidx/webkit/ProcessGlobalConfig.java
M webkit/webkit/src/main/java/androidx/webkit/WebViewFeature.java
M webkit/webkit/src/main/java/androidx/webkit/internal/StartupFeatures.java
M webkit/webkit/src/main/java/androidx/webkit/internal/WebViewFeatureInternal.java

Hash: 4fe72afdcf64cad8b2399ca7a28d1aa57a05f976
Date: Thu Oct 24 17:35:26 2024

Message last modified on Feb 22, 2025 02:40PM

Issue 388100796