From: csagan5 <32685696+csagan5@users.noreply.github.com> Date: Sat, 4 Dec 2021 11:41:31 +0100 Subject: Restore offline-indicator-v2 flag Reverts acc8b6f4542703211e5f3c5181914fd3374c9e84 License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html --- .../indicator/OfflineDetector.java | 38 ++++++++++++++++++- .../tabbed_mode/TabbedRootUiCoordinator.java | 8 +++- chrome/browser/about_flags.cc | 3 ++ chrome/browser/flag-metadata.json | 5 +++ chrome/browser/flag_descriptions.cc | 4 ++ chrome/browser/flag_descriptions.h | 3 ++ .../flags/android/chrome_feature_list.cc | 5 +++ .../flags/android/chrome_feature_list.h | 1 + .../browser/flags/ChromeFeatureList.java | 1 + 9 files changed, 65 insertions(+), 3 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/indicator/OfflineDetector.java b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/indicator/OfflineDetector.java --- a/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/indicator/OfflineDetector.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/indicator/OfflineDetector.java @@ -8,6 +8,7 @@ import android.content.Context; import android.os.Handler; import android.os.SystemClock; import android.provider.Settings; +import android.text.TextUtils; import androidx.annotation.VisibleForTesting; @@ -16,8 +17,10 @@ import org.chromium.base.ApplicationStatus; import org.chromium.base.Callback; import org.chromium.base.Log; import org.chromium.base.supplier.Supplier; +import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.net.connectivitydetector.ConnectivityDetector; import org.chromium.chrome.browser.net.connectivitydetector.ConnectivityDetector.ConnectionState; +import org.chromium.components.variations.VariationsAssociatedData; import org.chromium.components.version_info.VersionInfo; /** @@ -125,8 +128,9 @@ class OfflineDetector mIsForegroundCallback = isForegroundCallback; mContext = context; mHandler = new Handler(); - mStatusIndicatorWaitOnSwitchOnlineToOfflineDurationMs = - STATUS_INDICATOR_WAIT_ON_SWITCH_ONLINE_TO_OFFLINE_DEFAULT_DURATION_MS; + mStatusIndicatorWaitOnSwitchOnlineToOfflineDurationMs = getIntParamValueOrDefault( + "STATUS_INDICATOR_WAIT_ON_SWITCH_ONLINE_TO_OFFLINE_DEFAULT_DURATION_MS", + STATUS_INDICATOR_WAIT_ON_SWITCH_ONLINE_TO_OFFLINE_DEFAULT_DURATION_MS); mUpdateOfflineStatusIndicatorDelayedRunnable = () -> { if (sLoggingEnabled) { @@ -356,4 +360,34 @@ class OfflineDetector timeNeededAfterConnectionChangeFromOnlineToOffline), timeNeededAfterConnectionChangeFromAirplaneToOffline)); } + + /** + * Returns the value for a Finch parameter, or the default value if no parameter + * exists in the current configuration. + * @param paramName The name of the Finch parameter (or command-line switch) to get a value + * for. + * @param defaultValue The default value to return when there's no param or switch. + * @return The value -- either the param or the default. + */ + private static long getIntParamValueOrDefault(String paramName, long defaultValue) { + String value; + + // May throw exception in tests. + try { + value = VariationsAssociatedData.getVariationParamValue( + ChromeFeatureList.OFFLINE_INDICATOR_V2, paramName); + } catch (java.lang.UnsupportedOperationException e) { + return defaultValue; + } + + if (!TextUtils.isEmpty(value)) { + try { + return Integer.parseInt(value); + } catch (NumberFormatException e) { + return defaultValue; + } + } + + return defaultValue; + } } diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java b/chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java --- a/chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedRootUiCoordinator.java @@ -837,7 +837,8 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator { private void initStatusIndicatorCoordinator(LayoutManagerImpl layoutManager) { // TODO(crbug.com/1035584): Disable on tablets for now as we need to do one or two extra // things for tablets. - if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(mActivity)) { + if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(mActivity) + || (!ChromeFeatureList.isEnabled(ChromeFeatureList.OFFLINE_INDICATOR_V2))) { return; } @@ -859,6 +860,11 @@ public class TabbedRootUiCoordinator extends RootUiCoordinator { mStatusIndicatorCoordinator.addObserver(mStatusIndicatorObserver); mStatusIndicatorCoordinator.addObserver(mStatusBarColorController); + // Don't initialize the offline indicator controller if the feature is disabled. + if (!ChromeFeatureList.isEnabled(ChromeFeatureList.OFFLINE_INDICATOR_V2)) { + return; + } + ObservableSupplierImpl isUrlBarFocusedSupplier = new ObservableSupplierImpl<>(); isUrlBarFocusedSupplier.set(mToolbarManager.isUrlBarFocused()); mUrlFocusChangeListener = new UrlFocusChangeListener() { diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc @@ -5450,6 +5450,9 @@ const FeatureEntry kFeatureEntries[] = { flag_descriptions::kOfflinePagesLivePageSharingName, flag_descriptions::kOfflinePagesLivePageSharingDescription, kOsAndroid, FEATURE_VALUE_TYPE(offline_pages::kOfflinePagesLivePageSharingFeature)}, + {"offline-indicator-v2", flag_descriptions::kOfflineIndicatorV2Name, + flag_descriptions::kOfflineIndicatorV2Description, kOsAndroid, + FEATURE_VALUE_TYPE(chrome::android::kOfflineIndicatorV2)}, {"offline-pages-auto-save", flag_descriptions::kOfflinePagesAutoSaveFeatureName, flag_descriptions::kOfflinePagesAutoSaveFeatureDescription, kOsAndroid, diff --git a/chrome/browser/flag-metadata.json b/chrome/browser/flag-metadata.json --- a/chrome/browser/flag-metadata.json +++ b/chrome/browser/flag-metadata.json @@ -6047,6 +6047,11 @@ "owners": [ "dom" ], "expiry_milestone": 125 }, + { + "name": "offline-indicator-v2", + "owners": [ "sinansahin@google.com", "twellington", "offline-dev" ], + "expiry_milestone": -1 + }, { "name": "offline-pages-live-page-sharing", "owners": [ "sclittle", "srsudar", "offline-dev" ], diff --git a/chrome/browser/flag_descriptions.cc b/chrome/browser/flag_descriptions.cc --- a/chrome/browser/flag_descriptions.cc +++ b/chrome/browser/flag_descriptions.cc @@ -4249,6 +4249,10 @@ const char kNotificationPermissionRationaleBottomSheetDescription[] = "flow. " "Only works with builds targeting Android T+."; +const char kOfflineIndicatorV2Name[] = "Offline indicator V2"; +const char kOfflineIndicatorV2Description[] = + "Show a persistent offline indicator when offline."; + const char kOfflinePagesLivePageSharingName[] = "Enables live page sharing of offline pages"; const char kOfflinePagesLivePageSharingDescription[] = diff --git a/chrome/browser/flag_descriptions.h b/chrome/browser/flag_descriptions.h --- a/chrome/browser/flag_descriptions.h +++ b/chrome/browser/flag_descriptions.h @@ -2504,6 +2504,9 @@ extern const char kNewTabSearchEngineUrlAndroidDescription[]; extern const char kNotificationPermissionRationaleName[]; extern const char kNotificationPermissionRationaleDescription[]; +extern const char kOfflineIndicatorV2Name[]; +extern const char kOfflineIndicatorV2Description[]; + extern const char kNotificationPermissionRationaleBottomSheetName[]; extern const char kNotificationPermissionRationaleBottomSheetDescription[]; diff --git a/chrome/browser/flags/android/chrome_feature_list.cc b/chrome/browser/flags/android/chrome_feature_list.cc --- a/chrome/browser/flags/android/chrome_feature_list.cc +++ b/chrome/browser/flags/android/chrome_feature_list.cc @@ -273,6 +273,7 @@ const base::Feature* const kFeaturesExposedToJava[] = { &kNewTabSearchEngineUrlAndroid, &kNotificationPermissionVariant, &kNotificationPermissionBottomSheet, + &kOfflineIndicatorV2, &kPageAnnotationsService, &kPreconnectOnTabCreation, &kInlineUpdateFlow, @@ -818,6 +819,10 @@ BASE_FEATURE(kEarlyInitializeStartupMetrics, BASE_FEATURE(kEmptyStates, "EmptyStates", base::FEATURE_ENABLED_BY_DEFAULT); +BASE_FEATURE(kOfflineIndicatorV2, + "OfflineIndicatorV2", + base::FEATURE_ENABLED_BY_DEFAULT); + BASE_FEATURE(kExperimentsForAgsa, "ExperimentsForAgsa", base::FEATURE_ENABLED_BY_DEFAULT); diff --git a/chrome/browser/flags/android/chrome_feature_list.h b/chrome/browser/flags/android/chrome_feature_list.h --- a/chrome/browser/flags/android/chrome_feature_list.h +++ b/chrome/browser/flags/android/chrome_feature_list.h @@ -132,6 +132,7 @@ BASE_DECLARE_FEATURE(kLensCameraAssistedSearch); BASE_DECLARE_FEATURE(kLensOnQuickActionSearchWidget); BASE_DECLARE_FEATURE(kLocationBarModelOptimizations); BASE_DECLARE_FEATURE(kNewTabSearchEngineUrlAndroid); +BASE_DECLARE_FEATURE(kOfflineIndicatorV2); BASE_DECLARE_FEATURE(kNotificationPermissionVariant); BASE_DECLARE_FEATURE(kNotificationPermissionBottomSheet); BASE_DECLARE_FEATURE(kOmahaMinSdkVersionAndroid); diff --git a/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java b/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java --- a/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java +++ b/chrome/browser/flags/android/java/src/org/chromium/chrome/browser/flags/ChromeFeatureList.java @@ -331,6 +331,7 @@ public abstract class ChromeFeatureList { public static final String NOTIFICATION_PERMISSION_VARIANT = "NotificationPermissionVariant"; public static final String NOTIFICATION_PERMISSION_BOTTOM_SHEET = "NotificationPermissionBottomSheet"; + public static final String OFFLINE_INDICATOR_V2 = "OfflineIndicatorV2"; public static final String OFFLINE_PAGES_DESCRIPTIVE_FAIL_STATUS = "OfflinePagesDescriptiveFailStatus"; public static final String OFFLINE_PAGES_DESCRIPTIVE_PENDING_STATUS = -- 2.25.1