Skip to content

Commit

Permalink
[Fixit] Cleanup RequestDesktopSiteForTablets flag
Browse files Browse the repository at this point in the history
Cleanup RequestDesktopSiteForTablets flag as we are experimenting rds
based on screen size in inches instead of window size in dp.

Bug: 1402539
Change-Id: Icd8aa57f7aed11ff17e37934ee2d9f2b894ce528
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4117455
Reviewed-by: Sinan Sahin <sinansahin@google.com>
Commit-Queue: Shu Yang <shuyng@google.com>
Reviewed-by: Gang Wu <gangwu@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1085185}
  • Loading branch information
Shu Yang authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent 8b1a52c commit ee1fb64
Show file tree
Hide file tree
Showing 18 changed files with 5 additions and 225 deletions.
Expand Up @@ -17,11 +17,9 @@
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import org.chromium.base.ApplicationStatus;
import org.chromium.base.MathUtils;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.tab.state.CriticalPersistedTabData;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
Expand All @@ -31,7 +29,6 @@
import org.chromium.components.content_settings.ContentSettingsType;
import org.chromium.content_public.browser.ContentFeatureList;
import org.chromium.content_public.browser.WebContents;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.base.WindowAndroid;
import org.chromium.ui.display.DisplayAndroidManager;
import org.chromium.url.GURL;
Expand All @@ -43,8 +40,6 @@
* Collection of utility methods that operates on Tab.
*/
public class TabUtils {
private static final String REQUEST_DESKTOP_SCREEN_WIDTH_PARAM = "screen_width_dp";

/**
* Define the callers of NavigationControllerImpl#setUseDesktopUserAgent.
*/
Expand Down Expand Up @@ -204,33 +199,6 @@ public static boolean readRequestDesktopSiteContentSettings(
}
}

/**
* Check if the tab is large enough for displaying desktop sites. This method will only check
* for tablets, if the device is a phone, will return false regardless of tab size.
* @param tab The tab to be checked if the size is large enough for desktop site.
* @return Whether or not the screen size is large enough for desktop sites.
*/
public static boolean isTabLargeEnoughForDesktopSite(Tab tab) {
if (!DeviceFormFactor.isNonMultiDisplayContextOnTablet(tab.getContext())) {
// The device is a phone, do not check the tab size.
return false;
}
Activity activity = getActivity(tab);
if (activity == null) {
// It is possible that we are in custom tabs or tests, and need to access the activity
// differently.
activity = ApplicationStatus.getLastTrackedFocusedActivity();
if (activity == null) return false;
}
int windowWidth = activity.getWindow().getDecorView().getWidth();
int minWidthForDesktopSite = ChromeFeatureList.getFieldTrialParamByFeatureAsInt(
ChromeFeatureList.REQUEST_DESKTOP_SITE_FOR_TABLETS,
REQUEST_DESKTOP_SCREEN_WIDTH_PARAM,
/* Set a very large size as default to serve as a disabled screen width. */ 4096);

return minWidthForDesktopSite <= windowWidth;
}

/**
* Check if Request Desktop Site global setting is enabled.
* @param profile The profile of the tab.
Expand Down
Expand Up @@ -314,11 +314,6 @@ protected boolean canShowAppBanners() {
return mDelegate.canShowAppBanners();
}

@CalledByNative
private boolean isTabLargeEnoughForDesktopSite() {
return TabUtils.isTabLargeEnoughForDesktopSite(mTab);
}

/**
* @return the WebAPK manifest scope. This gives frames within the scope increased privileges
* such as autoplaying media unmuted.
Expand Down
44 changes: 0 additions & 44 deletions chrome/browser/about_flags.cc
Expand Up @@ -2489,44 +2489,6 @@ const FeatureEntry::FeatureVariation
kPhotoPickerVideoSupportEnabledWithAnimatedThumbnails,
std::size(kPhotoPickerVideoSupportEnabledWithAnimatedThumbnails),
nullptr}};

// Request Desktop Site on Tablet by default variations.
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets100[] = {
{"screen_width_dp", "100"},
{"enabled", "true"}};
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets600[] = {
{"screen_width_dp", "600"},
{"enabled", "true"}};
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets768[] = {
{"screen_width_dp", "768"},
{"enabled", "true"}};
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets960[] = {
{"screen_width_dp", "960"},
{"enabled", "true"}};
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets1024[] = {
{"screen_width_dp", "1024"},
{"enabled", "true"}};
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets1280[] = {
{"screen_width_dp", "1280"},
{"enabled", "true"}};
const FeatureEntry::FeatureParam kRequestDesktopSiteForTablets1920[] = {
{"screen_width_dp", "1920"},
{"enabled", "true"}};
const FeatureEntry::FeatureVariation kRequestDesktopSiteForTabletsVariations[] =
{{"for 100dp+ screens", kRequestDesktopSiteForTablets100,
std::size(kRequestDesktopSiteForTablets100), nullptr},
{"for 600dp+ screens", kRequestDesktopSiteForTablets600,
std::size(kRequestDesktopSiteForTablets600), nullptr},
{"for 768dp+ screens", kRequestDesktopSiteForTablets768,
std::size(kRequestDesktopSiteForTablets768), nullptr},
{"for 960dp+ screens", kRequestDesktopSiteForTablets960,
std::size(kRequestDesktopSiteForTablets960), nullptr},
{"for 1024dp+ screens", kRequestDesktopSiteForTablets1024,
std::size(kRequestDesktopSiteForTablets1024), nullptr},
{"for 1280dp+ screens", kRequestDesktopSiteForTablets1280,
std::size(kRequestDesktopSiteForTablets1280), nullptr},
{"for 1920dp+ screens", kRequestDesktopSiteForTablets1920,
std::size(kRequestDesktopSiteForTablets1920), nullptr}};
#endif // BUILDFLAG(IS_ANDROID)

// TODO(crbug.com/991082,1015377): Remove after proper support for back/forward
Expand Down Expand Up @@ -5399,12 +5361,6 @@ const FeatureEntry kFeatureEntries[] = {
#endif // BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_ANDROID)
{"request-desktop-site-for-tablets",
flag_descriptions::kRequestDesktopSiteForTabletsName,
flag_descriptions::kRequestDesktopSiteForTabletsDescription, kOsAndroid,
FEATURE_WITH_PARAMS_VALUE_TYPE(features::kRequestDesktopSiteForTablets,
kRequestDesktopSiteForTabletsVariations,
"RequestDesktopSiteForTablets")},
{"revoke-notifications-permission-if-disabled-on-app-level",
flag_descriptions::kRevokeNotificationsPermissionIfDisabledOnAppLevelName,
flag_descriptions::
Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/android/tab_web_contents_delegate_android.cc
Expand Up @@ -580,15 +580,6 @@ bool TabWebContentsDelegateAndroid::CanShowAppBanners() const {
return Java_TabWebContentsDelegateAndroidImpl_canShowAppBanners(env, obj);
}

bool TabWebContentsDelegateAndroid::IsTabLargeEnoughForDesktopSite() const {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = GetJavaDelegate(env);
if (obj.is_null())
return false;
return Java_TabWebContentsDelegateAndroidImpl_isTabLargeEnoughForDesktopSite(
env, obj);
}

const GURL TabWebContentsDelegateAndroid::GetManifestScope() const {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = GetJavaDelegate(env);
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/android/tab_web_contents_delegate_android.h
Expand Up @@ -130,7 +130,6 @@ class TabWebContentsDelegateAndroid
bool IsNightModeEnabled() const;
bool IsForceDarkWebContentEnabled() const;
bool CanShowAppBanners() const;
bool IsTabLargeEnoughForDesktopSite() const;

// Returns true if this tab is currently presented in the context of custom
// tabs. Tabs can be moved between different activities so the returned value
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/chrome_content_browser_client.cc
Expand Up @@ -5362,7 +5362,6 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(

#if BUILDFLAG(IS_ANDROID)
std::string client_data_header;
bool is_tab_large_enough = false;
bool is_custom_tab = false;
if (frame_tree_node_id != content::RenderFrameHost::kNoFrameTreeNodeId) {
auto* web_contents = WebContents::FromFrameTreeNodeId(frame_tree_node_id);
Expand All @@ -5380,7 +5379,6 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
web_contents->GetDelegate())
: nullptr;
if (delegate) {
is_tab_large_enough = delegate->IsTabLargeEnoughForDesktopSite();
is_custom_tab = delegate->IsCustomTab();
}
}
Expand All @@ -5393,7 +5391,7 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
profile->GetPrefs()->GetString(prefs::kAllowedDomainsForApps)};
result.push_back(std::make_unique<GoogleURLLoaderThrottle>(
#if BUILDFLAG(IS_ANDROID)
client_data_header, is_tab_large_enough,
client_data_header,
#endif
std::move(dynamic_params)));

Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/flag_descriptions.cc
Expand Up @@ -3909,13 +3909,6 @@ const char kRequestDesktopSiteExceptionsDescription[] =
"An option in `Site settings` to request the desktop version of websites "
"based on site level settings.";

const char kRequestDesktopSiteForTabletsName[] =
"Request desktop site for tablets on Android";
const char kRequestDesktopSiteForTabletsDescription[] =
"Requests a desktop site, if the screen size is large enough on Android."
" On tablets with small screens a mobile site will be requested by "
"default.";

const char kRequestDesktopSiteZoomName[] =
"Default zoom for request desktop site on Android.";
const char kRequestDesktopSiteZoomDescription[] =
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/flag_descriptions.h
Expand Up @@ -2239,9 +2239,6 @@ extern const char kRequestDesktopSiteExceptionsDescription[];
extern const char kRequestDesktopSiteExceptionsDowngradeName[];
extern const char kRequestDesktopSiteExceptionsDowngradeDescription[];

extern const char kRequestDesktopSiteForTabletsName[];
extern const char kRequestDesktopSiteForTabletsDescription[];

extern const char kRequestDesktopSiteZoomName[];
extern const char kRequestDesktopSiteZoomDescription[];

Expand Down
1 change: 0 additions & 1 deletion chrome/browser/flags/android/chrome_feature_list.cc
Expand Up @@ -125,7 +125,6 @@ const base::Feature* const kFeaturesExposedToJava[] = {
&features::kPwaUpdateDialogForIcon,
&features::kPwaUpdateDialogForName,
&features::kQuietNotificationPrompts,
&features::kRequestDesktopSiteForTablets,
&features::kToolbarUseHardwareBitmapDraw,
&features::kWebNfc,
&features::kIncognitoDownloadsWarning,
Expand Down
Expand Up @@ -464,7 +464,6 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
"RequestDesktopSiteOptInSynthetic";
public static final String REQUEST_DESKTOP_SITE_OPT_IN_CONTROL_SYNTHETIC =
"RequestDesktopSiteOptInControlSynthetic";
public static final String REQUEST_DESKTOP_SITE_FOR_TABLETS = "RequestDesktopSiteForTablets";
public static final String SAFE_BROWSING_DELAYED_WARNINGS = "SafeBrowsingDelayedWarnings";
public static final String SAFE_MODE_FOR_CACHED_FLAGS = "SafeModeForCachedFlags";
public static final String SCREENSHOTS_FOR_ANDROID_V2 = "ScreenshotsForAndroidV2";
Expand Down
6 changes: 0 additions & 6 deletions chrome/common/chrome_features.cc
Expand Up @@ -966,12 +966,6 @@ BASE_FEATURE(kRemoveSupervisedUsersOnStartup,
base::FEATURE_DISABLED_BY_DEFAULT);
#endif

#if BUILDFLAG(IS_ANDROID)
BASE_FEATURE(kRequestDesktopSiteForTablets,
"RequestDesktopSiteForTablets",
base::FEATURE_DISABLED_BY_DEFAULT);
#endif

#if !BUILDFLAG(IS_ANDROID)
// Enables notification permission module in Safety Check.
BASE_FEATURE(kSafetyCheckNotificationPermissions,
Expand Down
5 changes: 0 additions & 5 deletions chrome/common/chrome_features.h
Expand Up @@ -548,11 +548,6 @@ COMPONENT_EXPORT(CHROME_FEATURES)
BASE_DECLARE_FEATURE(kRemoveSupervisedUsersOnStartup);
#endif

#if BUILDFLAG(IS_ANDROID)
COMPONENT_EXPORT(CHROME_FEATURES)
BASE_DECLARE_FEATURE(kRequestDesktopSiteForTablets);
#endif

#if !BUILDFLAG(IS_ANDROID)
COMPONENT_EXPORT(CHROME_FEATURES)
BASE_DECLARE_FEATURE(kSafetyCheckNotificationPermissions);
Expand Down
20 changes: 0 additions & 20 deletions chrome/common/google_url_loader_throttle.cc
Expand Up @@ -15,10 +15,6 @@
#include "services/network/public/mojom/url_response_head.mojom.h"
#include "services/network/public/mojom/x_frame_options.mojom.h"

#if BUILDFLAG(IS_ANDROID)
#include "ui/base/device_form_factor.h"
#endif

#if BUILDFLAG(ENABLE_EXTENSIONS)
#include "extensions/common/extension_urls.h"
#endif
Expand All @@ -27,7 +23,6 @@ namespace {

#if BUILDFLAG(IS_ANDROID)
const char kCCTClientDataHeader[] = "X-CCT-Client-Data";
const char kRequestDesktopDataHeader[] = "X-Eligible-Tablet";
#endif

} // namespace
Expand All @@ -47,13 +42,11 @@ void GoogleURLLoaderThrottle::UpdateCorsExemptHeader(
GoogleURLLoaderThrottle::GoogleURLLoaderThrottle(
#if BUILDFLAG(IS_ANDROID)
const std::string& client_data_header,
bool is_tab_large_enough,
#endif
chrome::mojom::DynamicParams dynamic_params)
:
#if BUILDFLAG(IS_ANDROID)
client_data_header_(client_data_header),
is_tab_large_enough_(is_tab_large_enough),
#endif
dynamic_params_(std::move(dynamic_params)) {
}
Expand Down Expand Up @@ -97,19 +90,6 @@ void GoogleURLLoaderThrottle::WillStartRequest(
request->cors_exempt_headers.SetHeader(kCCTClientDataHeader,
client_data_header_);
}

bool is_google_homepage_or_search =
google_util::IsGoogleHomePageUrl(request->url) ||
google_util::IsGoogleSearchUrl(request->url);
if (is_google_homepage_or_search) {
if (base::FeatureList::IsEnabled(features::kRequestDesktopSiteForTablets) &&
ui::GetDeviceFormFactor() == ui::DEVICE_FORM_FACTOR_TABLET) {
request->headers.SetHeader(kRequestDesktopDataHeader,
is_tab_large_enough_ ? "1" : "0");
base::UmaHistogramBoolean("Android.RequestDesktopSite.TabletEligible",
is_tab_large_enough_);
}
}
#endif
}

Expand Down
2 changes: 0 additions & 2 deletions chrome/common/google_url_loader_throttle.h
Expand Up @@ -20,7 +20,6 @@ class GoogleURLLoaderThrottle
public:
#if BUILDFLAG(IS_ANDROID)
GoogleURLLoaderThrottle(const std::string& client_data_header,
bool is_tab_large_enough,
chrome::mojom::DynamicParams dynamic_params);
#else
explicit GoogleURLLoaderThrottle(chrome::mojom::DynamicParams dynamic_params);
Expand Down Expand Up @@ -51,7 +50,6 @@ class GoogleURLLoaderThrottle
private:
#if BUILDFLAG(IS_ANDROID)
std::string client_data_header_;
bool is_tab_large_enough_;
#endif
const chrome::mojom::DynamicParams dynamic_params_;
};
Expand Down

0 comments on commit ee1fb64

Please sign in to comment.