Commit eeb03d31 authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

content: adds RenderFrameMetadataObserverClient.OnRootScrollOffsetChanged

As part of making GestureStateListener.onScrollOffsetOrExtentChange
work again (
https://chromium-review.googlesource.com/c/chromium/src/+/2222623 )
I made it so that the browser gets any change in RenderFrameMetaData
when an infobar is present and the root-scroll-offset changes.
Unfortunately this regressed some perf tests. The test are triggering
an infobar to show, and then scrolling through the whole page without
dismissing. Uma stats indicate this isn't that uncommon, so it seems
worth fixing.

This patch creates a one off message (OnRootScrollOffsetChanged)
that is sent if root scrolls have been requested and the frame would
not normally be sent to the client.

https://pinpoint-dot-chromeperf.appspot.com/job/139b3471120000
is a pinpoint run that shows this gives back some portion of the
loss. In order to get back all the performance, a more complex
patch is likely needed. Given branch is Thursday, I'm landing this
patch now to give back some of the performance.


BUG=1091451

Change-Id: I54cc55a6dab1dc716978eb7d25cb932bfb662ee5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2257407Reviewed-by: default avatarBo <boliu@chromium.org>
Reviewed-by: default avatarkylechar <kylechar@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781951}
parent 58ea3835
...@@ -229,6 +229,17 @@ void GestureListenerManager::UpdateOnTouchDown() { ...@@ -229,6 +229,17 @@ void GestureListenerManager::UpdateOnTouchDown() {
Java_GestureListenerManagerImpl_updateOnTouchDown(env, obj); Java_GestureListenerManagerImpl_updateOnTouchDown(env, obj);
} }
void GestureListenerManager::OnRootScrollOffsetChanged(
const gfx::Vector2dF& root_scroll_offset) {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
if (obj.is_null())
return;
Java_GestureListenerManagerImpl_onRootScrollOffsetChanged(
env, obj, root_scroll_offset.x(), root_scroll_offset.y());
}
void GestureListenerManager::UpdateRenderProcessConnection( void GestureListenerManager::UpdateRenderProcessConnection(
RenderWidgetHostViewAndroid* old_rwhva, RenderWidgetHostViewAndroid* old_rwhva,
RenderWidgetHostViewAndroid* new_rwhva) { RenderWidgetHostViewAndroid* new_rwhva) {
......
...@@ -62,6 +62,7 @@ class CONTENT_EXPORT GestureListenerManager : public RenderWidgetHostConnector { ...@@ -62,6 +62,7 @@ class CONTENT_EXPORT GestureListenerManager : public RenderWidgetHostConnector {
const float top_shown_pix, const float top_shown_pix,
bool top_changed); bool top_changed);
void UpdateOnTouchDown(); void UpdateOnTouchDown();
void OnRootScrollOffsetChanged(const gfx::Vector2dF& root_scroll_offset);
// RendetWidgetHostConnector implementation. // RendetWidgetHostConnector implementation.
void UpdateRenderProcessConnection( void UpdateRenderProcessConnection(
......
...@@ -127,4 +127,10 @@ void RenderFrameMetadataProviderImpl::OnFrameSubmissionForTesting( ...@@ -127,4 +127,10 @@ void RenderFrameMetadataProviderImpl::OnFrameSubmissionForTesting(
weak_factory_.GetWeakPtr())); weak_factory_.GetWeakPtr()));
} }
void RenderFrameMetadataProviderImpl::OnRootScrollOffsetChanged(
const gfx::Vector2dF& root_scroll_offset) {
for (Observer& observer : observers_)
observer.OnRootScrollOffsetChanged(root_scroll_offset);
}
} // namespace content } // namespace content
...@@ -76,6 +76,8 @@ class CONTENT_EXPORT RenderFrameMetadataProviderImpl ...@@ -76,6 +76,8 @@ class CONTENT_EXPORT RenderFrameMetadataProviderImpl
uint32_t frame_token, uint32_t frame_token,
const cc::RenderFrameMetadata& metadata) override; const cc::RenderFrameMetadata& metadata) override;
void OnFrameSubmissionForTesting(uint32_t frame_token) override; void OnFrameSubmissionForTesting(uint32_t frame_token) override;
void OnRootScrollOffsetChanged(
const gfx::Vector2dF& root_scroll_offset) override;
base::ObserverList<Observer>::Unchecked observers_; base::ObserverList<Observer>::Unchecked observers_;
......
...@@ -580,6 +580,12 @@ void RenderWidgetHostViewAndroid:: ...@@ -580,6 +580,12 @@ void RenderWidgetHostViewAndroid::
RenderWidgetHostViewBase::OnRenderFrameMetadataChangedAfterActivation(); RenderWidgetHostViewBase::OnRenderFrameMetadataChangedAfterActivation();
} }
void RenderWidgetHostViewAndroid::OnRootScrollOffsetChanged(
const gfx::Vector2dF& root_scroll_offset) {
if (gesture_listener_manager_)
gesture_listener_manager_->OnRootScrollOffsetChanged(root_scroll_offset);
}
void RenderWidgetHostViewAndroid::Focus() { void RenderWidgetHostViewAndroid::Focus() {
if (view_.HasFocus()) if (view_.HasFocus())
GotFocus(); GotFocus();
......
...@@ -317,6 +317,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid ...@@ -317,6 +317,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid
void OnRenderFrameMetadataChangedBeforeActivation( void OnRenderFrameMetadataChangedBeforeActivation(
const cc::RenderFrameMetadata& metadata) override; const cc::RenderFrameMetadata& metadata) override;
void OnRenderFrameMetadataChangedAfterActivation() override; void OnRenderFrameMetadataChangedAfterActivation() override;
void OnRootScrollOffsetChanged(
const gfx::Vector2dF& root_scroll_offset) override;
void WasEvicted(); void WasEvicted();
......
...@@ -103,10 +103,15 @@ struct RenderFrameMetadata { ...@@ -103,10 +103,15 @@ struct RenderFrameMetadata {
// which a fully populated RenderFrameMetadata object (above) is delivered to // which a fully populated RenderFrameMetadata object (above) is delivered to
// the RenderFrameMetadataObserverClient. // the RenderFrameMetadataObserverClient.
interface RenderFrameMetadataObserver { interface RenderFrameMetadataObserver {
// When |enabled| is set to true, this will send RenderFrameMetadata to // When |enabled| is set to true this notifies the client of any change to the
// the RenderFrameMetadataObserverClient for any frame in which the root // root scroll offset. The client is notified in two ways:
// scroll changes. Used only on Android for acessibility or // . OnRenderFrameMetadataChanged(), is sent if the client would normally be
// GestureListenerManager. // notified of the frame (for example, the viewport changed).
// . OnRootScrollOffsetChanged() if the client is not notified of the frame
// change, but the root scroll offset has changed. In other words, if this
// is sent, *only* the root-scroll-offset has changed and the client is not
// sent a OnRenderFrameMetadataChanged() for the frame.
// Used on Android for acessibility and GestureListenerManager.
[EnableIf=is_android] [EnableIf=is_android]
ReportAllRootScrolls(bool enabled); ReportAllRootScrolls(bool enabled);
...@@ -127,4 +132,8 @@ interface RenderFrameMetadataObserverClient { ...@@ -127,4 +132,8 @@ interface RenderFrameMetadataObserverClient {
// Notified on all frame submissions. // Notified on all frame submissions.
OnFrameSubmissionForTesting(uint32 frame_token); OnFrameSubmissionForTesting(uint32 frame_token);
// Only called if ReportAllRootScrolls(true) has been called. See
// ReportAllRootScrolls() for details.
OnRootScrollOffsetChanged(gfx.mojom.Vector2dF root_scroll_offset);
}; };
...@@ -334,23 +334,47 @@ public class GestureListenerManagerImpl ...@@ -334,23 +334,47 @@ public class GestureListenerManagerImpl
|| scrollOffsetY != rc.getScrollY(); || scrollOffsetY != rc.getScrollY();
if (scrollChanged) { if (scrollChanged) {
mScrollDelegate.onScrollChanged((int) rc.fromLocalCssToPix(scrollOffsetX), onRootScrollOffsetChangedImpl(pageScaleFactor, scrollOffsetX, scrollOffsetY);
(int) rc.fromLocalCssToPix(scrollOffsetY), (int) rc.getScrollXPix(),
(int) rc.getScrollYPix());
} }
// TODO(jinsukkim): Consider updating the info directly through RenderCoordinates. // TODO(jinsukkim): Consider updating the info directly through RenderCoordinates.
rc.updateFrameInfo(scrollOffsetX, scrollOffsetY, contentWidth, contentHeight, viewportWidth, rc.updateFrameInfo(contentWidth, contentHeight, viewportWidth, viewportHeight,
viewportHeight, pageScaleFactor, minPageScaleFactor, maxPageScaleFactor, minPageScaleFactor, maxPageScaleFactor, topBarShownPix);
topBarShownPix);
if (scrollChanged || topBarChanged) { if (!scrollChanged && topBarChanged) {
updateOnScrollChanged(verticalScrollOffset(), verticalScrollExtent()); updateOnScrollChanged(verticalScrollOffset(), verticalScrollExtent());
} }
if (scaleLimitsChanged) updateOnScaleLimitsChanged(minPageScaleFactor, maxPageScaleFactor); if (scaleLimitsChanged) updateOnScaleLimitsChanged(minPageScaleFactor, maxPageScaleFactor);
TraceEvent.end("GestureListenerManagerImpl:updateScrollInfo"); TraceEvent.end("GestureListenerManagerImpl:updateScrollInfo");
} }
@SuppressWarnings("unused")
@CalledByNative
private void onRootScrollOffsetChanged(float scrollOffsetX, float scrollOffsetY) {
onRootScrollOffsetChangedImpl(mWebContents.getRenderCoordinates().getPageScaleFactor(),
scrollOffsetX, scrollOffsetY);
}
private void onRootScrollOffsetChangedImpl(
float pageScaleFactor, float scrollOffsetX, float scrollOffsetY) {
TraceEvent.begin("GestureListenerManagerImpl:onRootScrollOffsetChanged");
notifyDelegateOfScrollChange(scrollOffsetX, scrollOffsetY);
mWebContents.getRenderCoordinates().updateScrollInfo(
pageScaleFactor, scrollOffsetX, scrollOffsetY);
updateOnScrollChanged(verticalScrollOffset(), verticalScrollExtent());
TraceEvent.end("GestureListenerManagerImpl:onRootScrollOffsetChanged");
}
private void notifyDelegateOfScrollChange(float scrollOffsetX, float scrollOffsetY) {
RenderCoordinatesImpl rc = mWebContents.getRenderCoordinates();
mScrollDelegate.onScrollChanged((int) rc.fromLocalCssToPix(scrollOffsetX),
(int) rc.fromLocalCssToPix(scrollOffsetY), (int) rc.getScrollXPix(),
(int) rc.getScrollYPix());
}
@Override @Override
@CalledByNative @CalledByNative
public boolean isScrollInProgress() { public boolean isScrollInProgress() {
......
...@@ -98,13 +98,9 @@ public class RenderCoordinatesImpl implements RenderCoordinates { ...@@ -98,13 +98,9 @@ public class RenderCoordinatesImpl implements RenderCoordinates {
mDeviceScaleFactor = dipScale; mDeviceScaleFactor = dipScale;
} }
public void updateFrameInfo(float scrollXCss, float scrollYCss, float contentWidthCss, public void updateFrameInfo(float contentWidthCss, float contentHeightCss,
float contentHeightCss, float viewportWidthCss, float viewportHeightCss, float viewportWidthCss, float viewportHeightCss, float minPageScaleFactor,
float pageScaleFactor, float minPageScaleFactor, float maxPageScaleFactor, float maxPageScaleFactor, float contentOffsetYPix) {
float contentOffsetYPix) {
mScrollXCss = scrollXCss;
mScrollYCss = scrollYCss;
mPageScaleFactor = pageScaleFactor;
mMinPageScaleFactor = minPageScaleFactor; mMinPageScaleFactor = minPageScaleFactor;
mMaxPageScaleFactor = maxPageScaleFactor; mMaxPageScaleFactor = maxPageScaleFactor;
mTopContentOffsetYPix = contentOffsetYPix; mTopContentOffsetYPix = contentOffsetYPix;
...@@ -114,6 +110,12 @@ public class RenderCoordinatesImpl implements RenderCoordinates { ...@@ -114,6 +110,12 @@ public class RenderCoordinatesImpl implements RenderCoordinates {
mLastFrameViewportHeightCss = viewportHeightCss; mLastFrameViewportHeightCss = viewportHeightCss;
} }
public void updateScrollInfo(float pageScaleFactor, float scrollXCss, float scrollYCss) {
mPageScaleFactor = pageScaleFactor;
mScrollXCss = scrollXCss;
mScrollYCss = scrollYCss;
}
/** /**
* @return Horizontal scroll offset in CSS pixels. * @return Horizontal scroll offset in CSS pixels.
*/ */
......
...@@ -40,6 +40,8 @@ class CONTENT_EXPORT RenderFrameMetadataProvider { ...@@ -40,6 +40,8 @@ class CONTENT_EXPORT RenderFrameMetadataProvider {
// to pass in Viz. // to pass in Viz.
virtual void OnLocalSurfaceIdChanged( virtual void OnLocalSurfaceIdChanged(
const cc::RenderFrameMetadata& metadata) = 0; const cc::RenderFrameMetadata& metadata) = 0;
virtual void OnRootScrollOffsetChanged(
const gfx::Vector2dF& root_scroll_offset) {}
}; };
RenderFrameMetadataProvider() = default; RenderFrameMetadataProvider() = default;
......
...@@ -58,6 +58,12 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission( ...@@ -58,6 +58,12 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission(
send_metadata |= force_send; send_metadata |= force_send;
} }
const bool send_root_scroll_offset_changed =
!send_metadata && last_render_frame_metadata_ &&
last_render_frame_metadata_->root_scroll_offset !=
render_frame_metadata.root_scroll_offset &&
render_frame_metadata.root_scroll_offset.has_value();
// Always cache the full metadata, so that it can correctly be sent upon // Always cache the full metadata, so that it can correctly be sent upon
// ReportAllFrameSubmissionsForTesting or on android, which notifies on any // ReportAllFrameSubmissionsForTesting or on android, which notifies on any
// root scroll offset change. This must only be done after we've compared the // root scroll offset change. This must only be done after we've compared the
...@@ -97,6 +103,10 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission( ...@@ -97,6 +103,10 @@ void RenderFrameMetadataObserverImpl::OnRenderFrameSubmission(
? metadata_copy.local_surface_id_allocation->local_surface_id() ? metadata_copy.local_surface_id_allocation->local_surface_id()
.ToString() .ToString()
: "null"); : "null");
} else if (render_frame_metadata_observer_client_ &&
send_root_scroll_offset_changed) {
render_frame_metadata_observer_client_->OnRootScrollOffsetChanged(
*render_frame_metadata.root_scroll_offset);
} }
// Always cache the initial frame token, so that if a test connects later on // Always cache the initial frame token, so that if a test connects later on
...@@ -159,9 +169,6 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata( ...@@ -159,9 +169,6 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
} }
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
bool need_send_root_scroll =
report_all_root_scrolls_enabled_ &&
rfm1.root_scroll_offset != rfm2.root_scroll_offset;
if (rfm1.bottom_controls_height != rfm2.bottom_controls_height || if (rfm1.bottom_controls_height != rfm2.bottom_controls_height ||
rfm1.bottom_controls_shown_ratio != rfm2.bottom_controls_shown_ratio || rfm1.bottom_controls_shown_ratio != rfm2.bottom_controls_shown_ratio ||
rfm1.top_controls_min_height_offset != rfm1.top_controls_min_height_offset !=
...@@ -173,8 +180,7 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata( ...@@ -173,8 +180,7 @@ bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
rfm1.root_overflow_y_hidden != rfm2.root_overflow_y_hidden || rfm1.root_overflow_y_hidden != rfm2.root_overflow_y_hidden ||
rfm1.scrollable_viewport_size != rfm2.scrollable_viewport_size || rfm1.scrollable_viewport_size != rfm2.scrollable_viewport_size ||
rfm1.root_layer_size != rfm2.root_layer_size || rfm1.root_layer_size != rfm2.root_layer_size ||
rfm1.has_transparent_background != rfm2.has_transparent_background || rfm1.has_transparent_background != rfm2.has_transparent_background) {
need_send_root_scroll) {
*needs_activation_notification = true; *needs_activation_notification = true;
return true; return true;
} }
......
...@@ -42,6 +42,7 @@ class MockRenderFrameMetadataObserverClient ...@@ -42,6 +42,7 @@ class MockRenderFrameMetadataObserverClient
void(uint32_t frame_token, void(uint32_t frame_token,
const cc::RenderFrameMetadata& metadata)); const cc::RenderFrameMetadata& metadata));
MOCK_METHOD1(OnFrameSubmissionForTesting, void(uint32_t frame_token)); MOCK_METHOD1(OnFrameSubmissionForTesting, void(uint32_t frame_token));
MOCK_METHOD1(OnRootScrollOffsetChanged, void(const gfx::Vector2dF& offset));
private: private:
mojo::Receiver<mojom::RenderFrameMetadataObserverClient> mojo::Receiver<mojom::RenderFrameMetadataObserverClient>
...@@ -223,10 +224,8 @@ TEST_F(RenderFrameMetadataObserverImplTest, SendRootScrollsForAccessibility) { ...@@ -223,10 +224,8 @@ TEST_F(RenderFrameMetadataObserverImplTest, SendRootScrollsForAccessibility) {
false /* force_send */); false /* force_send */);
{ {
base::RunLoop run_loop; base::RunLoop run_loop;
// The 0u frame token indicates that the client should not expect EXPECT_CALL(client(), OnRootScrollOffsetChanged(
// a corresponding frame token from Viz. *(render_frame_metadata.root_scroll_offset)))
EXPECT_CALL(client(), OnRenderFrameMetadataChanged(expected_frame_token,
render_frame_metadata))
.WillOnce(InvokeClosure(run_loop.QuitClosure())); .WillOnce(InvokeClosure(run_loop.QuitClosure()));
run_loop.Run(); run_loop.Run();
} }
......
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