Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -43358,6 +43358,7 @@ ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/Flu
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterViewDelegate.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/KeyChannelResponder.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/KeyData.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java + ../../../flutter/LICENSE
Expand Down Expand Up @@ -43477,6 +43478,7 @@ ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/FlutterMain.java
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArguments.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/FlutterViewDelegate.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/TextureRegistry.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/io/flutter/view/VsyncWaiter.java + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/android/jni/jni_mock.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -46250,6 +46252,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/Flutt
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterSurfaceView.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterTextureView.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterView.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/FlutterViewDelegate.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/KeyChannelResponder.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/KeyData.java
FILE: ../../../flutter/shell/platform/android/io/flutter/embedding/android/KeyEmbedderResponder.java
Expand Down Expand Up @@ -46379,6 +46382,7 @@ FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterMain.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterRunArguments.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterView.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/FlutterViewDelegate.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/TextureRegistry.java
FILE: ../../../flutter/shell/platform/android/io/flutter/view/VsyncWaiter.java
FILE: ../../../flutter/shell/platform/android/jni/jni_mock.h
Expand Down
1 change: 1 addition & 0 deletions shell/platform/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ android_java_sources = [
"io/flutter/embedding/android/FlutterSurfaceView.java",
"io/flutter/embedding/android/FlutterTextureView.java",
"io/flutter/embedding/android/FlutterView.java",
"io/flutter/embedding/android/FlutterViewDelegate.java",
"io/flutter/embedding/android/KeyChannelResponder.java",
"io/flutter/embedding/android/KeyData.java",
"io/flutter/embedding/android/KeyEmbedderResponder.java",
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Digging into go/customizable-window-headers I found APPEARANCE_TRANSPARENT_CAPTION_BAR_BACKGROUND Can you test an app with that flag set vs the default and come up with a recommendation on if we should apply transparent caption bar by default or not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should not be considered blocking.

Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,13 @@ public void removeFlutterEngineAttachmentListener(
.send();
}

private FlutterViewDelegate delegate = new FlutterViewDelegate();

@VisibleForTesting
public void setDelegate(FlutterViewDelegate delegate) {
this.delegate = delegate;
}

private void sendViewportMetricsToFlutter() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the design of this class I am seeing a pattern of the flutterView having methods that trigger on different android lifecycle events, Then ViewportMetrics being updated then at the end of that triggering method calling sendViewportMetricsToFlutter.

Looking at this code we do the calculation every send. When I was searching internally I did not find documentation of a lifecycle method to trigger on so maybe this is moot. If this code moved to onApplyWindowInsets would it still work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested this, and it looks good. I've moved it to onApplyWindowInsets after the existing calculations.

if (!isAttachedToFlutterEngine()) {
Log.w(
Expand All @@ -1460,6 +1467,17 @@ private void sendViewportMetricsToFlutter() {

viewportMetrics.devicePixelRatio = getResources().getDisplayMetrics().density;
viewportMetrics.physicalTouchSlop = ViewConfiguration.get(getContext()).getScaledTouchSlop();

if (Build.VERSION.SDK_INT >= API_LEVELS.API_35) {
List<Rect> boundingRects = delegate.getCaptionBarInsets(getContext());
if (boundingRects != null && boundingRects.size() == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it safe to ignore the boundingRects at positions 2 and beyond?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had assumed the caption bar would have only one rect. If this is false, what would you suppose we do if there are more, considering that the viewport metrics describe only a bounding rect for the view? Get the maximum to use for the padding value?

Copy link
Contributor

@reidbaker reidbaker Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can sometimes work from assumptions. I am asking what evidence justifies ignoring that the api returns a list. I dont know if bounding recs are a single value or list. That is your responsibility as the author of a change.

When/if we would get multiple recs would help us decide what to do? Assuming we dont have that information then I would think the largest value is probably the one to choose.

viewportMetrics.viewPaddingTop =
Math.max(boundingRects.get(0).bottom, viewportMetrics.viewPaddingTop);
}
} else {
Log.w(TAG, "API level " + Build.VERSION.SDK_INT + " is too low to query bounding rects.");
}

flutterEngine.getRenderer().setViewportMetrics(viewportMetrics);
}

Expand All @@ -1485,6 +1503,15 @@ public void setVisibility(int visibility) {
}
}

/**
* Allow access to the viewport metrics so that tests can set them to be valid with nonzero
* dimensions.
*/
@VisibleForTesting
public FlutterRenderer.ViewportMetrics getViewportMetrics() {
return viewportMetrics;
}

/**
* Listener that is notified when a {@link io.flutter.embedding.engine.FlutterEngine} is attached
* to/detached from a given {@code FlutterView}.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

package io.flutter.embedding.android;

import android.app.Activity;
import android.content.Context;
import android.graphics.Rect;
import android.view.Window;
import android.view.WindowInsets;
import androidx.annotation.RequiresApi;
import androidx.annotation.VisibleForTesting;
import io.flutter.Build;
import io.flutter.util.ViewUtils;
import java.util.Collections;
import java.util.List;

/** A delegate class that performs the task of retrieving the bounding rect values. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything you can add here to let future maintainers know when logic should go into FlutterViewDelegate vs sendViewportMetricsToFlutter

public class FlutterViewDelegate {
/**
* Return the WindowInsets object for the provided Context, or null if there is no associated
* activity.
*/
@RequiresApi(api = Build.API_LEVELS.API_23)
@VisibleForTesting
public WindowInsets getWindowInsets(Context context) {
Activity activity = ViewUtils.getActivity(context);
if (activity == null) {
return null;
}
Window window = activity.getWindow();
if (window == null) {
return null;
}
return window.getDecorView().getRootWindowInsets();
}

@RequiresApi(api = Build.API_LEVELS.API_35)
@VisibleForTesting
public List<Rect> getCaptionBarInsets(Context context) {
WindowInsets insets = getWindowInsets(context);
if (insets == null) {
return Collections.emptyList();
}
return insets.getBoundingRects(WindowInsets.Type.captionBar());
}
}