8343398: Add reducedData preference #1656

mstr2 · 2024-12-03T15:17:19Z

The reducedData preference instructs applications to minimize internet traffic, as users might be on a metered network or a limited data plan.

This corresponds to the following OS settings:

Windows: Settings -> Network and Internet -> Ethernet/WiFi -> Metered connection
macOS: Settings -> Network -> Ethernet/WiFi -> Network Settings -> Low data mode
Ubuntu: Settings -> Network -> Wired/WiFi -> Metered connection

Change notifications work consistently on Windows and macOS. On my Ubuntu 24 system, the GIO network-changed signal is not sent when I only toggle the "metered connection" flag in network settings (and there's no signal specifically for low-data mode). The new value is only picked up when the connection changes by coming offline or going online.

/reviewers 2
/csr

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)
Change requires CSR request JDK-8345677 to be approved

Issues

JDK-8343398: Add reducedData preference (Enhancement - P4)
JDK-8345677: Add reducedData preference (CSR)

Reviewers

Andy Goryachev (@andy-goryachev-oracle - Reviewer) 🔄 Re-review required (review applies to 455613f0)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/1656/head:pull/1656
$ git checkout pull/1656

Update a local copy of the PR:
$ git checkout pull/1656
$ git pull https://git.openjdk.org/jfx.git pull/1656/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1656

View PR using the GUI difftool:
$ git pr show -t 1656

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/1656.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-12-03T15:18:00Z

👋 Welcome back mstrauss! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2024-12-03T15:18:23Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2024-12-03T15:18:57Z

@mstr2
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

openjdk · 2024-12-03T15:19:29Z

@mstr2 has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@mstr2 please create a CSR request for issue JDK-8343398 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

mlbridge · 2024-12-03T15:23:15Z

Webrevs

mstr2 · 2024-12-04T15:59:26Z

This PR also includes a small refactor of the GTK and macOS PlatformSupport classes.

The previous implementations had some parts of the change notification mechanism implemented outside of PlatformSupport in their respective GlassApplication class. The refactor moves this into PlatformSupport, which centralizes all aspects related to platform preference change detection.

andy-goryachev-oracle · 2024-12-11T16:02:27Z

Does not seem to work on macOS 15.1.1 M1:

andy-goryachev-oracle · 2024-12-11T16:05:02Z

mstr2 · 2024-12-11T16:15:05Z

Does not seem to work on macOS 15.1.1 M1:

It works on my 14.6 machine. Let me try upgrading and testing again...

andy-goryachev-oracle · 2024-12-11T16:17:04Z

I might suggest adding the instructions on how to control the platform settings as comments somewhere in the implementation.

Windows: Settings -> Network and Internet -> Ethernet/WiFi -> Metered connection
macOS: Settings -> Network -> Ethernet/WiFi -> Network Settings -> Low data mode
Ubuntu: Settings -> Network -> Wired/WiFi -> Metered connection

andy-goryachev-oracle · 2024-12-11T16:17:42Z

(I've merged the latest master prior to testing)

mstr2 · 2024-12-11T17:44:11Z

Does not seem to work on macOS 15.1.1 M1:

It also works on 15.1.1 on my machine. Just to check the obvious: did you close the WiFi Details window? The settings are only applied when you click OK.

andy-goryachev-oracle · 2024-12-11T17:58:01Z

Update: I was getting interference from the VPN application. Getting off the VPN helped - I see the property and macOS.*.currentPathConstrained key being toggled.

(macOS.*.currentPathExpensive remains false).

mstr2 · 2024-12-11T18:10:17Z

Update: I was getting interference from the VPN application. Getting off the VPN helped - I see the property and macOS.*.currentPathConstrained key being toggled.

(macOS.*.currentPathExpensive remains false).

That's the expected result.

mstr2 · 2024-12-11T18:14:40Z

I might suggest adding the instructions on how to control the platform settings as comments somewhere in the implementation.
Windows: Settings -> Network and Internet -> Ethernet/WiFi -> Metered connection
macOS: Settings -> Network -> Ethernet/WiFi -> Network Settings -> Low data mode
Ubuntu: Settings -> Network -> Wired/WiFi -> Metered connection

I feel this wouldn't age well, considering that the particular UI used by the different operating systems has often changed over the years.

andy-goryachev-oracle

It looks like the scope of this change goes far beyond adding a few properties. It looks like a non-trivial refactoring that might also impact other platform preferences.

I did check that switching between light/dark modes on macOS results in proper updates, but the native code needs to be re-reviewed.

andy-goryachev-oracle · 2024-12-11T18:33:10Z

modules/javafx.graphics/src/main/native-glass/mac/GlassApplication.m

@@ -204,6 +187,7 @@ - (void)applicationWillFinishLaunching:(NSNotification *)aNotification
    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
    {
        // unblock main thread. Glass is started at this point.
+        self->platformSupport = [[PlatformSupport alloc] initWithEnv:env application:jApplication];


should we check for failure here? (alloc)

We never do that in obj-c native code, so this probably wouldn't be the place to start. In any case, a failure at this point wouldn't lead to a crash, as the only access of platformSupport checks for nil.

andy-goryachev-oracle · 2024-12-11T20:08:50Z

I feel this wouldn't age well, considering that the particular UI used by the different operating systems has often changed over the years.

I think it has merit. Now that we need to re-test the functionality, it would have helped. Even when something changes, it will be easier to look up a few missing pieces than dig through the PRs...

kevinrushforth · 2024-12-11T20:27:36Z

I feel this wouldn't age well, considering that the particular UI used by the different operating systems has often changed over the years.

I think it has merit. Now that we need to re-test the functionality, it would have helped. Even when something changes, it will be easier to look up a few missing pieces than dig through the PRs...

I think this sort of information is better suited for a Wiki.

Btw, I can review the native code changes. Just not for a couple days.

andy-goryachev-oracle · 2024-12-11T20:33:43Z

tested macOS behavior, all is well.

BTW, I've updated the Monkey Tester to highlight changes in Tools -> Platform Preferences Monitor.

https://github.com/andy-goryachev-oracle/MonkeyTest

andy-goryachev-oracle

tested on macOS. needs testing on linux and windows.

kevinrushforth · 2024-12-11T20:39:26Z

BTW, I've updated the Monkey Tester to highlight changes in Tools -> Platform Preferences Monitor.

https://github.com/andy-goryachev-oracle/MonkeyTest

That seems useful. Thanks.

tested on macOS. needs testing on linux and windows.

I can test Windows and Linux (I'll skip macOS since you've already done that).

mstr2 added 4 commits December 3, 2024 01:59

Implementation of reducedData

b473f3e

add linux implementation

9d22732

add macOS implementation

fe22bd5

error message tweak

0245c2e

openjdk bot added the rfr Ready for review label Dec 3, 2024

openjdk bot added the csr Need approved CSR to integrate pull request label Dec 3, 2024

mstr2 added 2 commits December 4, 2024 01:39

javadoc change

5c2e255

refactor macOS PlatformSupport

8b8b1bd

Merge branch 'master' into feature/reduced-data

455613f

andy-goryachev-oracle reviewed Dec 11, 2024

View reviewed changes

kevinrushforth self-requested a review December 11, 2024 20:25

andy-goryachev-oracle approved these changes Dec 11, 2024

View reviewed changes

changed formatting

4bfcb33

mstr2 requested a review from andy-goryachev-oracle December 11, 2024 21:59

formatting

cbd3cb6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8343398: Add reducedData preference #1656

8343398: Add reducedData preference #1656

mstr2 commented Dec 3, 2024 •

edited by openjdk bot

Loading

bridgekeeper bot commented Dec 3, 2024

openjdk bot commented Dec 3, 2024

openjdk bot commented Dec 3, 2024

openjdk bot commented Dec 3, 2024

mlbridge bot commented Dec 3, 2024 •

edited

Loading

mstr2 commented Dec 4, 2024

andy-goryachev-oracle commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

mstr2 commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

mstr2 commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

mstr2 commented Dec 11, 2024

mstr2 commented Dec 11, 2024

andy-goryachev-oracle left a comment

andy-goryachev-oracle Dec 11, 2024

mstr2 Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

kevinrushforth commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

andy-goryachev-oracle left a comment

kevinrushforth commented Dec 11, 2024

8343398: Add reducedData preference #1656

Are you sure you want to change the base?

8343398: Add reducedData preference #1656

Conversation

mstr2 commented Dec 3, 2024 • edited by openjdk bot Loading

Progress

Issues

Reviewers

Reviewing

bridgekeeper bot commented Dec 3, 2024

openjdk bot commented Dec 3, 2024

openjdk bot commented Dec 3, 2024

openjdk bot commented Dec 3, 2024

mlbridge bot commented Dec 3, 2024 • edited Loading

Webrevs

mstr2 commented Dec 4, 2024

andy-goryachev-oracle commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

mstr2 commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

mstr2 commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

mstr2 commented Dec 11, 2024

mstr2 commented Dec 11, 2024

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

andy-goryachev-oracle Dec 11, 2024

Choose a reason for hiding this comment

mstr2 Dec 11, 2024

Choose a reason for hiding this comment

andy-goryachev-oracle commented Dec 11, 2024

kevinrushforth commented Dec 11, 2024

andy-goryachev-oracle commented Dec 11, 2024

andy-goryachev-oracle left a comment

Choose a reason for hiding this comment

kevinrushforth commented Dec 11, 2024

mstr2 commented Dec 3, 2024 •

edited by openjdk bot

Loading

mlbridge bot commented Dec 3, 2024 •

edited

Loading