Commit 81c89fc5 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

[Android] Disable inset sizing adjustments when showing fullscreen video.

BUG=852297

Change-Id: Ia18c775059b61c90e2c10050699ebb256233c7b1
Reviewed-on: https://chromium-review.googlesource.com/1106423
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569844}
parent dac2bad4
......@@ -6,7 +6,6 @@ package org.chromium.chrome.browser;
import android.app.Notification;
import android.content.Intent;
import android.os.Build;
import android.os.Handler;
import android.os.Looper;
import android.support.annotation.Nullable;
......@@ -275,16 +274,6 @@ public abstract class AppHooks {
ContextCompat.startForegroundService(ContextUtils.getApplicationContext(), intent);
}
/**
* @return Whether the renderer should detect whether video elements are in fullscreen. The
* detection results can be retrieved through
* {@link WebContents.hasActiveEffectivelyFullscreenVideo()}.
*/
@CalledByNative
public boolean shouldDetectVideoFullscreen() {
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O;
}
/**
* @return A callback that will be run each time an offline page is saved in the custom tabs
* namespace.
......
......@@ -103,7 +103,7 @@ public class CompositorViewHolder extends FrameLayout
private ControlContainer mControlContainer;
private InsetObserverView mInsetObserverView;
private boolean mSystemUiFullscreen;
private boolean mShowingFullscreenNonVideoContent;
private Runnable mSystemUiFullscreenResizeRunnable;
/** The currently visible Tab. */
......@@ -255,7 +255,7 @@ public class CompositorViewHolder extends FrameLayout
// contents.
//
// [1] - https://developer.android.com/reference/android/view/WindowManager.LayoutParams.html#FLAG_FULLSCREEN
if (mSystemUiFullscreen) {
if (mShowingFullscreenNonVideoContent) {
getWindowVisibleDisplayFrame(mCacheRect);
// On certain devices, getWindowVisibleDisplayFrame is larger than the screen size, so
......@@ -269,7 +269,7 @@ public class CompositorViewHolder extends FrameLayout
private int getHeightForViewport() {
// See comment in getWidthForViewport() for an explainer of why this is done.
if (mSystemUiFullscreen) {
if (mShowingFullscreenNonVideoContent) {
getWindowVisibleDisplayFrame(mCacheRect);
return Math.min(mCacheRect.height(), getHeight());
} else {
......@@ -293,8 +293,21 @@ public class CompositorViewHolder extends FrameLayout
view = (View) view.getParent();
}
if (mSystemUiFullscreen == isInFullscreen) return;
mSystemUiFullscreen = isInFullscreen;
if (!isInFullscreen && !mShowingFullscreenNonVideoContent) return;
if (isInFullscreen && mFullscreenManager != null
&& mFullscreenManager.getPersistentFullscreenMode()) {
Tab tab = getCurrentTab();
WebContents webContents = tab != null ? tab.getWebContents() : null;
// When playing fullscreen video, the inset size adjustments are always temporary and
// should be ignored to avoid relayout janks.
if (webContents != null && webContents.hasActiveEffectivelyFullscreenVideo()) {
isInFullscreen = false;
}
}
if (mShowingFullscreenNonVideoContent == isInFullscreen) return;
mShowingFullscreenNonVideoContent = isInFullscreen;
if (mSystemUiFullscreenResizeRunnable == null) {
mSystemUiFullscreenResizeRunnable = () -> {
......@@ -369,7 +382,7 @@ public class CompositorViewHolder extends FrameLayout
@Override
public void onInsetChanged(int top, int left, int bottom, int right) {
if (mSystemUiFullscreen) onViewportChanged();
if (mShowingFullscreenNonVideoContent) onViewportChanged();
}
@Override
......
......@@ -19,7 +19,6 @@ import org.chromium.base.Callback;
import org.chromium.base.Log;
import org.chromium.base.library_loader.LibraryLoader;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.AppHooks;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.rappor.RapporServiceBridge;
......@@ -103,8 +102,7 @@ public class PictureInPictureController {
if (!ChromeFeatureList.isEnabled(ChromeFeatureList.VIDEO_PERSISTENCE)) return false;
// Only auto-PiP if there is a playing fullscreen video that allows PiP.
if (!AppHooks.get().shouldDetectVideoFullscreen()
|| !webContents.hasActiveEffectivelyFullscreenVideo()
if (!webContents.hasActiveEffectivelyFullscreenVideo()
|| !webContents.isPictureInPictureAllowedForFullscreenVideo()) {
recordAttemptResult(METRICS_ATTEMPT_RESULT_NO_VIDEO);
return false;
......
......@@ -17,7 +17,6 @@ import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.FlakyTest;
import org.chromium.base.test.util.MinAndroidSdkLevel;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.fullscreen.FullscreenOptions;
......@@ -99,13 +98,10 @@ public class FullscreenVideoTest {
}
/**
* Tests that the dimensions of the fullscreen video are propagated correctly. Tracking the
* dimensions of fullscreen video is only enabled on Android O devices at time of writing
* (see {@link org.chromium.chrome.browser.AppHooks#shouldDetectVideoFullscreen()}).
* Tests that the dimensions of the fullscreen video are propagated correctly.
*/
@Test
@MediumTest
@MinAndroidSdkLevel(26)
public void testFullscreenDimensions() throws InterruptedException, TimeoutException {
String url =
mTestServerRule.getServer().getURL("/content/test/data/media/video-player.html");
......
......@@ -16,13 +16,6 @@ AppHooks::AppHooks() = default;
AppHooks::~AppHooks() = default;
bool AppHooks::ShouldDetectVideoFullscreen() {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> app_hooks_obj = Java_AppHooks_get(env);
return Java_AppHooks_shouldDetectVideoFullscreen(env, app_hooks_obj);
}
ScopedJavaLocalRef<jobject> AppHooks::GetOfflinePagesCCTRequestDoneCallback() {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> app_hooks_obj = Java_AppHooks_get(env);
......
......@@ -14,7 +14,6 @@ namespace android {
class AppHooks {
public:
static bool ShouldDetectVideoFullscreen();
static base::android::ScopedJavaLocalRef<jobject>
GetOfflinePagesCCTRequestDoneCallback();
......
......@@ -3063,8 +3063,7 @@ void ChromeContentBrowserClient::OverrideWebkitPrefs(
}
#if defined(OS_ANDROID)
web_prefs->video_fullscreen_detection_enabled =
chrome::android::AppHooks::ShouldDetectVideoFullscreen();
web_prefs->video_fullscreen_detection_enabled = true;
web_prefs->enable_media_download_in_product_help =
base::FeatureList::IsEnabled(
......
......@@ -435,24 +435,20 @@ public interface WebContents extends Parcelable {
/**
* Whether the WebContents has an active fullscreen video with native or custom controls.
* The WebContents must be fullscreen when this method is called. Fullscreen videos may take a
* moment to register. This should only be called if AppHooks.shouldDetectVideoFullscreen()
* returns true.
* moment to register.
*/
boolean hasActiveEffectivelyFullscreenVideo();
/**
* Whether the WebContents is allowed to enter Picture-in-Picture when it has an active
* fullscreen video with native or custom controls.
* This should only be called if AppHooks.shouldDetectVideoFullscreen()
* returns true.
*/
boolean isPictureInPictureAllowedForFullscreenVideo();
/**
* Gets a Rect containing the size of the currently playing fullscreen video. The position of
* the rectangle is meaningless. Will return null if there is no such video. Fullscreen videos
* may take a moment to register. This should only be called if
* AppHooks.shouldDetectVideoFullscreen() returns true.
* may take a moment to register.
*/
@Nullable
Rect getFullscreenVideoSize();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment