Skip to content

Commit

Permalink
Revert "[Paint Preview] Cleanup Show@Startup Flag"
Browse files Browse the repository at this point in the history
This reverts commit 491178e.

Reason for revert: Causing failure on CI. Lint was not updated in 
original CL. 
https://ci.chromium.org/ui/p/chromium/builders/try/android_compile_dbg/1313435/overview

Please update lint: https://chromium.googlesource.com/chromium/src/+/main/build/android/docs/lint.md

Original change's description:
> [Paint Preview] Cleanup Show@Startup Flag
>
> Cleanup flags for the PaintPreview Show@Startup experiment including
> configurations, and params.
>
> This still keeps a number of metrics active for periodic health
> monitoring.
>
> Bug: 1321121
> Change-Id: I4f62f0fdf53f6ba880cf53618610b9d75014a0b5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3995277
> Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
> Reviewed-by: Xi Han <hanxi@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1066598}

Bug: 1321121
Change-Id: If2d2e718a5a6f2f1d8d2343e3dae4aad357b0ebf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4004999
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Hailey Wang <haileywang@google.com>
Auto-Submit: Hailey Wang <haileywang@google.com>
Cr-Commit-Position: refs/heads/main@{#1067348}
  • Loading branch information
Hailey Wang authored and Chromium LUCI CQ committed Nov 4, 2022
1 parent db0147b commit 95d10e8
Show file tree
Hide file tree
Showing 16 changed files with 98 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ public static PseudoTab getActiveTabFromStateFile(Context context) {
}

private static void readAllPseudoTabsFromStateFile(Context context) {
assert ChromeFeatureList.sInstantStart.isEnabled();
assert ChromeFeatureList.sInstantStart.isEnabled()
|| ChromeFeatureList.sPaintPreviewShowOnStartup.isEnabled();
if (sReadStateFile) return;
sReadStateFile = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.chromium.chrome.browser.omnibox.OmniboxFeatures;
import org.chromium.chrome.browser.optimization_guide.OptimizationGuidePushNotificationManager;
import org.chromium.chrome.browser.page_annotations.PageAnnotationsServiceConfig;
import org.chromium.chrome.browser.paint_preview.StartupPaintPreviewHelper;
import org.chromium.chrome.browser.paint_preview.services.PaintPreviewTabService;
import org.chromium.chrome.browser.tab.state.FilePersistedTabDataStorage;
import org.chromium.chrome.browser.tabmodel.TabPersistentStore;
import org.chromium.chrome.browser.tasks.ConditionalTabStripUtils;
Expand Down Expand Up @@ -116,6 +118,7 @@ public void cacheNativeFlags() {
add(ChromeFeatureList.sOptimizationGuidePushNotifications);
add(ChromeFeatureList.sOSKResizesVisualViewportByDefault);
add(ChromeFeatureList.sPaintPreviewDemo);
add(ChromeFeatureList.sPaintPreviewShowOnStartup);
add(ChromeFeatureList.sQueryTiles);
add(ChromeFeatureList.sQueryTilesOnStart);
add(ChromeFeatureList.sReadLater);
Expand Down Expand Up @@ -182,6 +185,8 @@ public void cacheNativeFlags() {
add(StartSurfaceConfiguration.SUPPORT_ACCESSIBILITY);
add(StartSurfaceConfiguration.TAB_COUNT_BUTTON_ON_START_SURFACE);
add(StartSurfaceConfiguration.USER_CLICK_THRESHOLD);
add(StartupPaintPreviewHelper.ACCESSIBILITY_SUPPORT_PARAM);
add(PaintPreviewTabService.ALLOW_SRP);
add(TabContentManager.ALLOW_TO_REFETCH_TAB_THUMBNAIL_VARIATION);
add(TabPersistentStore.CRITICAL_PERSISTED_TAB_DATA_SAVE_ONLY_PARAM);
add(TabUiFeatureUtilities.ENABLE_LAUNCH_BUG_FIX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import org.chromium.base.ObserverList;
import org.chromium.base.supplier.ObservableSupplier;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.flags.BooleanCachedFieldTrialParameter;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.fullscreen.BrowserControlsManager;
import org.chromium.chrome.browser.metrics.PageLoadMetrics;
import org.chromium.chrome.browser.metrics.UmaUtils;
Expand All @@ -22,13 +24,17 @@
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorObserver;
import org.chromium.chrome.browser.toolbar.load_progress.LoadProgressCoordinator;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.WindowAndroid;

/**
* Glue code for the Paint Preview show-on-startup feature.
*/
public class StartupPaintPreviewHelper {
public static final BooleanCachedFieldTrialParameter ACCESSIBILITY_SUPPORT_PARAM =
new BooleanCachedFieldTrialParameter(ChromeFeatureList.PAINT_PREVIEW_SHOW_ON_STARTUP,
"has_accessibility_support", true);
/**
* Tracks whether a paint preview should be shown on tab restore. We use this to only attempt
* to display a paint preview on the first tab restoration that happens on Chrome startup when
Expand Down Expand Up @@ -110,7 +116,7 @@ private boolean preventShowOnRestore(Tab tab) {
* @return the feature availability
*/
public static boolean isEnabled() {
return true;
return ChromeFeatureList.sPaintPreviewShowOnStartup.isEnabled();
}

/**
Expand All @@ -134,6 +140,11 @@ public static void showPaintPreviewOnRestore(Tab tab) {
return;
}

if (ChromeAccessibilityUtil.get().isAccessibilityEnabled()
&& !ACCESSIBILITY_SUPPORT_PARAM.getValue()) {
return;
}

sShouldShowOnRestore = false;
LoadProgressCoordinator loadProgressCoordinator =
paintPreviewHelper.mProgressBarCoordinatorSupplier.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.chromium.chrome.browser.paint_preview.services.PaintPreviewTabServiceFactory;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.browser.Features;
import org.chromium.content_public.browser.test.util.TestThreadUtils;

import java.util.concurrent.ExecutionException;
Expand All @@ -35,6 +36,7 @@
* {@link ChromeActivity}.
*/
@RunWith(StartupPaintPreviewHelperTestRunner.class)
@Features.EnableFeatures({ChromeFeatureList.PAINT_PREVIEW_SHOW_ON_STARTUP})
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
@ParameterizedCommandLineFlags({
@Switches("disable-features=" + ChromeFeatureList.START_SURFACE_ANDROID + ","
Expand Down
14 changes: 14 additions & 0 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2958,6 +2958,15 @@ constexpr char kWallpaperGooglePhotosIntegrationInternalName[] =
constexpr char kWallpaperPerDeskName[] = "per-desk-wallpaper";
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(ENABLE_PAINT_PREVIEW) && BUILDFLAG(IS_ANDROID)
const FeatureEntry::FeatureParam kPaintPreviewStartupWithAccessibility[] = {
{"has_accessibility_support", "true"}};

const FeatureEntry::FeatureVariation kPaintPreviewStartupVariations[] = {
{"with accessibility support", kPaintPreviewStartupWithAccessibility,
std::size(kPaintPreviewStartupWithAccessibility), nullptr}};
#endif // BUILDFLAG(ENABLE_PAINT_PREVIEW) && BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_CHROMEOS_ASH)
constexpr char kBorealisBigGlInternalName[] = "borealis-big-gl";
constexpr char kBorealisDiskManagementInternalName[] =
Expand Down Expand Up @@ -7646,6 +7655,11 @@ const FeatureEntry kFeatureEntries[] = {
{"paint-preview-demo", flag_descriptions::kPaintPreviewDemoName,
flag_descriptions::kPaintPreviewDemoDescription, kOsAndroid,
FEATURE_VALUE_TYPE(paint_preview::kPaintPreviewDemo)},
{"paint-preview-startup", flag_descriptions::kPaintPreviewStartupName,
flag_descriptions::kPaintPreviewStartupDescription, kOsAndroid,
FEATURE_WITH_PARAMS_VALUE_TYPE(paint_preview::kPaintPreviewShowOnStartup,
kPaintPreviewStartupVariations,
"PaintPreviewShowOnStartup")},
#endif // BUILDFLAG(ENABLE_PAINT_PREVIEW) && BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_ANDROID)
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/flag-metadata.json
Original file line number Diff line number Diff line change
Expand Up @@ -5478,6 +5478,11 @@
"owners": [ "ckitagawa", "fredmello", "chrome-fdt@google.com" ],
"expiry_milestone": 111
},
{
"name": "paint-preview-startup",
"owners": [ "ckitagawa", "fredmello", "chrome-fdt@google.com" ],
"expiry_milestone": 109
},
{
"name": "partial-split",
"owners": [ "sophiewen", "chromeos-wmp-eng@google.com" ],
Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6510,6 +6510,12 @@ const char kPaintPreviewDemoName[] = "Paint Preview Demo";
const char kPaintPreviewDemoDescription[] =
"If enabled a menu item is added to the Android main menu to demo paint "
"previews.";
const char kPaintPreviewStartupName[] = "Paint Preview Startup";
const char kPaintPreviewStartupDescription[] =
"If enabled, paint previews for each tab are captured when a tab is hidden "
"and are deleted when a tab is closed. If a paint preview was captured for "
"the tab to be restored on startup, the paint preview will be shown "
"instead.";
#endif // ENABLE_PAINT_PREVIEW && BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -3764,6 +3764,8 @@ extern const char kEnableOopPrintDriversDescription[];
#if BUILDFLAG(ENABLE_PAINT_PREVIEW) && BUILDFLAG(IS_ANDROID)
extern const char kPaintPreviewDemoName[];
extern const char kPaintPreviewDemoDescription[];
extern const char kPaintPreviewStartupName[];
extern const char kPaintPreviewStartupDescription[];
#endif // ENABLE_PAINT_PREVIEW && BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(ENABLE_WEBUI_TAB_STRIP)
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/flags/android/chrome_feature_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&features::kDnsOverHttps,
&notifications::features::kUseChimeAndroidSdk,
&paint_preview::kPaintPreviewDemo,
&paint_preview::kPaintPreviewShowOnStartup,
&language::kAppLanguagePrompt,
&language::kAppLanguagePromptULP,
&language::kDetailedLanguageSettings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,8 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
public static final CachedFlag sOSKResizesVisualViewportByDefault =
new CachedFlag(OSK_RESIZES_VISUAL_VIEWPORT, false);
public static final CachedFlag sPaintPreviewDemo = new CachedFlag(PAINT_PREVIEW_DEMO, false);
public static final CachedFlag sPaintPreviewShowOnStartup =
new CachedFlag(PAINT_PREVIEW_SHOW_ON_STARTUP, false);
public static final CachedFlag sPrefetchNotificationSchedulingIntegration =
new CachedFlag(PREFETCH_NOTIFICATION_SCHEDULING_INTEGRATION, false);
public static final CachedFlag sQueryTiles = new CachedFlag(QUERY_TILES, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.chromium.base.Callback;
import org.chromium.base.supplier.Supplier;
import org.chromium.chrome.browser.browser_controls.BrowserStateBrowserControlsVisibilityDelegate;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.paint_preview.StartupPaintPreviewMetrics.ExitCause;
import org.chromium.chrome.browser.paint_preview.StartupPaintPreviewMetrics.PaintPreviewMetricsObserver;
import org.chromium.chrome.browser.tab.EmptyTabObserver;
Expand Down Expand Up @@ -56,6 +57,7 @@ public class StartupPaintPreview implements PlayerManager.Listener {
private Supplier<Boolean> mIsOfflinePage;

private static final int DEFAULT_INITIAL_REMOVE_DELAY_MS = 0;
private static final String INITIAL_REMOVE_DELAY_PARAM = "initial_remove_delay_ms";
private static final int SNACKBAR_DURATION_MS = 8 * 1000;

@IntDef({
Expand Down Expand Up @@ -227,12 +229,16 @@ public void onWebContentsFirstMeaningfulPaint(WebContents webContents) {

if (mState != State.SHOWING) return;

long delayMs = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.PAINT_PREVIEW_SHOW_ON_STARTUP, INITIAL_REMOVE_DELAY_PARAM,
DEFAULT_INITIAL_REMOVE_DELAY_MS);
// Delay removing paint preview after didFirstVisuallyNonEmptyPaint and no user
// interaction by |delayMs|. This is to account for 'heavy' pages that take a while
// to finish painting and avoid having flickers when switching from paint preview
// to the live page.
new Handler().postDelayed(
() -> { remove(ExitCause.TAB_FINISHED_LOADING); }, DEFAULT_INITIAL_REMOVE_DELAY_MS);
new Handler().postDelayed(() -> {
remove(ExitCause.TAB_FINISHED_LOADING);
}, delayMs);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.base.task.PostTask;
import org.chromium.chrome.browser.flags.BooleanCachedFieldTrialParameter;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver;
Expand All @@ -34,6 +36,10 @@
*/
@JNINamespace("paint_preview")
public class PaintPreviewTabService implements NativePaintPreviewServiceProvider {
public static final BooleanCachedFieldTrialParameter ALLOW_SRP =
new BooleanCachedFieldTrialParameter(
ChromeFeatureList.PAINT_PREVIEW_SHOW_ON_STARTUP, "allow_srp", false);

private static final long AUDIT_START_DELAY_MS = 2 * 60 * 1000; // Two minutes;
private static boolean sIsAccessibilityEnabledForTesting;

Expand Down Expand Up @@ -92,6 +98,8 @@ private boolean qualifiesForCapture(Tab tab) {
}

private boolean allowIfSrp(Tab tab) {
if (ALLOW_SRP.getValue()) return true;

return !UrlUtilitiesJni.get().isGoogleSearchUrl(tab.getUrl().getSpec());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void testCapturedAndDelete() throws Exception {

TestThreadUtils.runOnUiThreadBlocking(() -> {
mPaintPreviewTabService = PaintPreviewTabServiceFactory.getServiceInstance();
mPaintPreviewTabService.onRestoreCompleted(mTabModelSelector, true);
mTab.loadUrl(new LoadUrlParams(url));
});
// Give the tab time to complete layout before hiding.
Expand Down Expand Up @@ -125,6 +126,7 @@ public void testCapturedAndDeleteViaAudit() throws Exception {

TestThreadUtils.runOnUiThreadBlocking(() -> {
mPaintPreviewTabService = PaintPreviewTabServiceFactory.getServiceInstance();
mPaintPreviewTabService.onRestoreCompleted(mTabModelSelector, true);
mTab.loadUrl(new LoadUrlParams(url));
});
// Give the tab time to complete layout before hiding.
Expand Down
4 changes: 4 additions & 0 deletions components/paint_preview/features/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@ BASE_FEATURE(kPaintPreviewDemo,
"PaintPreviewDemo",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kPaintPreviewShowOnStartup,
"PaintPreviewShowOnStartup",
base::FEATURE_ENABLED_BY_DEFAULT);

} // namespace paint_preview
7 changes: 7 additions & 0 deletions components/paint_preview/features/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ namespace paint_preview {
// capturing and playing paint preview works on a specific site.
BASE_DECLARE_FEATURE(kPaintPreviewDemo);

// Used to enable the paint preview capture and show on startup for Android. If
// enabled, paint previews for each tab are captured when a tab is hidden and
// are deleted when a tab is closed. When a tab with a captured paint perview
// is shown at startup and there is no cached page we will show the paint
// preview.
BASE_DECLARE_FEATURE(kPaintPreviewShowOnStartup);

} // namespace paint_preview

#endif // COMPONENTS_PAINT_PREVIEW_FEATURES_FEATURES_H_
18 changes: 18 additions & 0 deletions testing/variations/fieldtrial_testing_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -8144,6 +8144,24 @@
]
}
],
"PaintPreviewShowOnStartup": [
{
"platforms": [
"android"
],
"experiments": [
{
"name": "Enabled_NoSrp_20220719",
"params": {
"allow_srp": "false"
},
"enable_features": [
"PaintPreviewShowOnStartup"
]
}
]
}
],
"ParkableImages": [
{
"platforms": [
Expand Down

0 comments on commit 95d10e8

Please sign in to comment.