Send addl RenderFrameMetadata if WebContentsAccessibilityAndroid set
When accessibility is in use we need per-frame scroll offset updates on
Android. This previously occurred with per-frame CompositorFrameMetadata
but is missing with selective RenderFrameMetadata.
This change causes us to always send RenderFrameMetadata for root scroll
offset changes if WebContentsAccessibilityAndroid is set.
TBR=ericrk@chromium.org
(cherry picked from commit 8c7ab9f25db763aed5cbf12ac0208f022a7269d4)
Bug: 964849, 967552
Change-Id: If310649ceccd2cc4b7f0d4110415dce5e8a7ce39
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1646092
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#669379}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1663399
Reviewed-by: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#396}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/content/browser/accessibility/web_contents_accessibility_android.cc b/content/browser/accessibility/web_contents_accessibility_android.cc
index 25af4dc..549931e 100644
--- a/content/browser/accessibility/web_contents_accessibility_android.cc
+++ b/content/browser/accessibility/web_contents_accessibility_android.cc
@@ -338,9 +338,9 @@
RenderWidgetHostViewAndroid* old_rwhva,
RenderWidgetHostViewAndroid* new_rwhva) {
if (old_rwhva)
- old_rwhva->set_web_contents_accessibility(nullptr);
+ old_rwhva->SetWebContentsAccessibility(nullptr);
if (new_rwhva)
- new_rwhva->set_web_contents_accessibility(accessibility_.get());
+ new_rwhva->SetWebContentsAccessibility(accessibility_.get());
}
WebContentsAccessibilityAndroid::WebContentsAccessibilityAndroid(
diff --git a/content/browser/renderer_host/render_frame_metadata_provider_impl.cc b/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
index 760bb72..9b7c9908 100644
--- a/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
+++ b/content/browser/renderer_host/render_frame_metadata_provider_impl.cc
@@ -35,16 +35,37 @@
render_frame_metadata_observer_client_binding_.Bind(std::move(client_request),
task_runner_);
- if (pending_report_all_frame_submission_.has_value()) {
- ReportAllFrameSubmissionsForTesting(*pending_report_all_frame_submission_);
- pending_report_all_frame_submission_.reset();
+#if defined(OS_ANDROID)
+ if (pending_report_all_root_scrolls_for_accessibility_.has_value()) {
+ ReportAllRootScrollsForAccessibility(
+ *pending_report_all_root_scrolls_for_accessibility_);
+ pending_report_all_root_scrolls_for_accessibility_.reset();
+ }
+#endif
+ if (pending_report_all_frame_submission_for_testing_.has_value()) {
+ ReportAllFrameSubmissionsForTesting(
+ *pending_report_all_frame_submission_for_testing_);
+ pending_report_all_frame_submission_for_testing_.reset();
}
}
+#if defined(OS_ANDROID)
+void RenderFrameMetadataProviderImpl::ReportAllRootScrollsForAccessibility(
+ bool enabled) {
+ if (!render_frame_metadata_observer_ptr_) {
+ pending_report_all_root_scrolls_for_accessibility_ = enabled;
+ return;
+ }
+
+ render_frame_metadata_observer_ptr_->ReportAllRootScrollsForAccessibility(
+ enabled);
+}
+#endif
+
void RenderFrameMetadataProviderImpl::ReportAllFrameSubmissionsForTesting(
bool enabled) {
if (!render_frame_metadata_observer_ptr_) {
- pending_report_all_frame_submission_ = enabled;
+ pending_report_all_frame_submission_for_testing_ = enabled;
return;
}
diff --git a/content/browser/renderer_host/render_frame_metadata_provider_impl.h b/content/browser/renderer_host/render_frame_metadata_provider_impl.h
index 2ec032d..6eeb0bc 100644
--- a/content/browser/renderer_host/render_frame_metadata_provider_impl.h
+++ b/content/browser/renderer_host/render_frame_metadata_provider_impl.h
@@ -8,6 +8,7 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
+#include "build/build_config.h"
#include "content/common/render_frame_metadata.mojom.h"
#include "content/public/browser/render_frame_metadata_provider.h"
#include "mojo/public/cpp/bindings/binding.h"
@@ -38,6 +39,12 @@
void Bind(mojom::RenderFrameMetadataObserverClientRequest client_request,
mojom::RenderFrameMetadataObserverPtr observer);
+#if defined(OS_ANDROID)
+ // Notifies the renderer to begin sending a notification on all root scroll
+ // changes, which is needed for accessibility on Android.
+ void ReportAllRootScrollsForAccessibility(bool enabled);
+#endif
+
// Notifies the renderer to begin sending a notification on all frame
// submissions.
void ReportAllFrameSubmissionsForTesting(bool enabled) override;
@@ -81,7 +88,10 @@
render_frame_metadata_observer_client_binding_;
mojom::RenderFrameMetadataObserverPtr render_frame_metadata_observer_ptr_;
- base::Optional<bool> pending_report_all_frame_submission_;
+#if defined(OS_ANDROID)
+ base::Optional<bool> pending_report_all_root_scrolls_for_accessibility_;
+#endif
+ base::Optional<bool> pending_report_all_frame_submission_for_testing_;
base::WeakPtrFactory<RenderFrameMetadataProviderImpl> weak_factory_;
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index f235629..6ac81d6 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -274,6 +274,9 @@
mojom::RenderFrameMetadataObserverClientPtrInfo client_info);
~FakeRenderFrameMetadataObserver() override {}
+#if defined(OS_ANDROID)
+ void ReportAllRootScrollsForAccessibility(bool enabled) override {}
+#endif
void ReportAllFrameSubmissionsForTesting(bool enabled) override {}
private:
diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc
index 297957c..be29174 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.cc
+++ b/content/browser/renderer_host/render_widget_host_view_android.cc
@@ -2523,4 +2523,15 @@
}
}
+void RenderWidgetHostViewAndroid::SetWebContentsAccessibility(
+ WebContentsAccessibilityAndroid* web_contents_accessibility) {
+ web_contents_accessibility_ = web_contents_accessibility;
+
+ if (host()) {
+ host()
+ ->render_frame_metadata_provider()
+ ->ReportAllRootScrollsForAccessibility(!!web_contents_accessibility_);
+ }
+}
+
} // namespace content
diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h
index c1cfc4c..881011539 100644
--- a/content/browser/renderer_host/render_widget_host_view_android.h
+++ b/content/browser/renderer_host/render_widget_host_view_android.h
@@ -350,6 +350,9 @@
void WasEvicted();
+ void SetWebContentsAccessibility(
+ WebContentsAccessibilityAndroid* web_contents_accessibility);
+
ui::DelegatedFrameHostAndroid* delegated_frame_host_for_testing() {
return delegated_frame_host_.get();
}
@@ -548,6 +551,8 @@
// either RenderFrameMetadata or CompositorFrameMetadata.
base::Optional<DevToolsFrameMetadata> last_devtools_frame_metadata_;
+ WebContentsAccessibilityAndroid* web_contents_accessibility_ = nullptr;
+
base::WeakPtrFactory<RenderWidgetHostViewAndroid> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewAndroid);
diff --git a/content/browser/renderer_host/render_widget_host_view_base.h b/content/browser/renderer_host/render_widget_host_view_base.h
index 903131f..81bdc91 100644
--- a/content/browser/renderer_host/render_widget_host_view_base.h
+++ b/content/browser/renderer_host/render_widget_host_view_base.h
@@ -78,7 +78,6 @@
class SyntheticGestureTarget;
class TextInputManager;
class TouchSelectionControllerClientManager;
-class WebContentsAccessibility;
class WebCursor;
class DelegatedFrameHost;
struct TextInputState;
@@ -565,10 +564,6 @@
bool is_fullscreen() { return is_fullscreen_; }
- void set_web_contents_accessibility(WebContentsAccessibility* wcax) {
- web_contents_accessibility_ = wcax;
- }
-
void set_is_currently_scrolling_viewport(
bool is_currently_scrolling_viewport) {
is_currently_scrolling_viewport_ = is_currently_scrolling_viewport;
@@ -660,8 +655,6 @@
// |content_background_color|.
base::Optional<SkColor> default_background_color_;
- WebContentsAccessibility* web_contents_accessibility_ = nullptr;
-
bool is_currently_scrolling_viewport_ = false;
private:
diff --git a/content/common/render_frame_metadata.mojom b/content/common/render_frame_metadata.mojom
index f1302d7..6b841dd 100644
--- a/content/common/render_frame_metadata.mojom
+++ b/content/common/render_frame_metadata.mojom
@@ -85,17 +85,26 @@
bool has_transparent_background;
};
-// This interface is provided by the renderer. It can optionally enable
-// notifications for all frame submissions.
+// This interface is provided by the renderer. It impacts the frequency with
+// which a fully populated RenderFrameMetadata object (above) is delivered to
+// the RenderFrameMetadataObserverClient.
interface RenderFrameMetadataObserver {
- // When |enabled| is set to true, this will notify the associated client of
- // all frame submissions.
+ // When |enabled| is set to true, this will send RenderFrameMetadata to
+ // the RenderFrameMetadataObserverClient for any frame in which the root
+ // scroll changes. Used only on Android for accessibility cases.
+ [EnableIf=is_android]
+ ReportAllRootScrollsForAccessibility(bool enabled);
+ // When |enabled| is set to true, this will send RenderFrameMetadata to
+ // the RenderFrameMetadataObserverClient for all frames. Only used for
+ // tests.
ReportAllFrameSubmissionsForTesting(bool enabled);
};
-// This interface is provided by the browser. It is notified of all changes to
+// This interface is provided by the browser. It is notified of changes to
// RenderFrameMetadata. It can be notified of all frame submissions, via
-// RenderFrameMetadataObserver::ReportAllFrameSubmissionsForTesting.
+// RenderFrameMetadataObserver::ReportAllFrameSubmissionsForTesting, or of
+// additional frames with root scroll offset changes via
+// RenderFrameMetadataObserver::ReportAllRootScrollsForAccessibility.
interface RenderFrameMetadataObserverClient {
// Notified when RenderFrameMetadata has changed.
OnRenderFrameMetadataChanged(uint32 frame_token, RenderFrameMetadata metadata);
diff --git a/content/renderer/render_frame_metadata_observer_impl.cc b/content/renderer/render_frame_metadata_observer_impl.cc
index 31fe9a0..6d72311 100644
--- a/content/renderer/render_frame_metadata_observer_impl.cc
+++ b/content/renderer/render_frame_metadata_observer_impl.cc
@@ -59,7 +59,8 @@
}
// Allways cache the full metadata, so that it can correctly be sent upon
- // ReportAllFrameSubmissionsForTesting. This must only be done after we've
+ // ReportAllFrameSubmissionsForTesting or
+ // ReportAllRootScrollsForAccessibility. This must only be done after we've
// compared the two for changes.
last_render_frame_metadata_ = render_frame_metadata;
@@ -67,11 +68,11 @@
// generated for first time and same as the default value, update the default
// value to all the observers.
if (send_metadata && render_frame_metadata_observer_client_) {
- // Sending |root_scroll_offset| outside of tests would leave the browser
- // process with out of date information. It is an optional parameter
- // which we clear here.
auto metadata_copy = render_frame_metadata;
#if !defined(OS_ANDROID)
+ // On non-Android, sending |root_scroll_offset| outside of tests would
+ // leave the browser process with out of date information. It is an
+ // optional parameter which we clear here.
if (!report_all_frame_submissions_for_testing_enabled_)
metadata_copy.root_scroll_offset = base::nullopt;
#endif
@@ -107,11 +108,26 @@
}
}
+#if defined(OS_ANDROID)
+void RenderFrameMetadataObserverImpl::ReportAllRootScrollsForAccessibility(
+ bool enabled) {
+ report_all_root_scrolls_for_accessibility_enabled_ = enabled;
+
+ if (enabled)
+ SendLastRenderFrameMetadata();
+}
+#endif
+
void RenderFrameMetadataObserverImpl::ReportAllFrameSubmissionsForTesting(
bool enabled) {
report_all_frame_submissions_for_testing_enabled_ = enabled;
- if (!enabled || !last_frame_token_)
+ if (enabled)
+ SendLastRenderFrameMetadata();
+}
+
+void RenderFrameMetadataObserverImpl::SendLastRenderFrameMetadata() {
+ if (!last_frame_token_)
return;
// When enabled for testing send the cached metadata.
@@ -121,11 +137,10 @@
last_frame_token_, *last_render_frame_metadata_);
}
-// static
bool RenderFrameMetadataObserverImpl::ShouldSendRenderFrameMetadata(
const cc::RenderFrameMetadata& rfm1,
const cc::RenderFrameMetadata& rfm2,
- bool* needs_activation_notification) {
+ bool* needs_activation_notification) const {
if (rfm1.root_background_color != rfm2.root_background_color ||
rfm1.is_scroll_offset_at_top != rfm2.is_scroll_offset_at_top ||
rfm1.selection != rfm2.selection ||
@@ -142,6 +157,9 @@
}
#if defined(OS_ANDROID)
+ bool need_send_root_scroll =
+ report_all_root_scrolls_for_accessibility_enabled_ &&
+ rfm1.root_scroll_offset != rfm2.root_scroll_offset;
if (rfm1.bottom_controls_height != rfm2.bottom_controls_height ||
rfm1.bottom_controls_shown_ratio != rfm2.bottom_controls_shown_ratio ||
rfm1.min_page_scale_factor != rfm2.min_page_scale_factor ||
@@ -149,7 +167,8 @@
rfm1.root_overflow_y_hidden != rfm2.root_overflow_y_hidden ||
rfm1.scrollable_viewport_size != rfm2.scrollable_viewport_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;
return true;
}
diff --git a/content/renderer/render_frame_metadata_observer_impl.h b/content/renderer/render_frame_metadata_observer_impl.h
index fc830dc9..c0335ae9 100644
--- a/content/renderer/render_frame_metadata_observer_impl.h
+++ b/content/renderer/render_frame_metadata_observer_impl.h
@@ -5,6 +5,7 @@
#ifndef CONTENT_RENDERER_RENDER_FRAME_METADATA_OBSERVER_IMPL_H_
#define CONTENT_RENDERER_RENDER_FRAME_METADATA_OBSERVER_IMPL_H_
+#include "build/build_config.h"
#include "cc/trees/render_frame_metadata.h"
#include "cc/trees/render_frame_metadata_observer.h"
#include "content/common/content_export.h"
@@ -39,6 +40,9 @@
bool force_send) override;
// mojom::RenderFrameMetadataObserver:
+#if defined(OS_ANDROID)
+ void ReportAllRootScrollsForAccessibility(bool enabled) override;
+#endif
void ReportAllFrameSubmissionsForTesting(bool enabled) override;
private:
@@ -50,12 +54,19 @@
// |needs_activation_notification| indicates whether the browser process
// expects notification of activation of the assoicated CompositorFrame from
// Viz.
- static bool ShouldSendRenderFrameMetadata(
- const cc::RenderFrameMetadata& rfm1,
- const cc::RenderFrameMetadata& rfm2,
- bool* needs_activation_notification);
+ bool ShouldSendRenderFrameMetadata(const cc::RenderFrameMetadata& rfm1,
+ const cc::RenderFrameMetadata& rfm2,
+ bool* needs_activation_notification) const;
- // When true this will notifiy |render_frame_metadata_observer_client_| of all
+ void SendLastRenderFrameMetadata();
+
+#if defined(OS_ANDROID)
+ // When true this will notify |render_frame_metadata_observer_client_| of all
+ // frame submissions that involve a root scroll offset change.
+ bool report_all_root_scrolls_for_accessibility_enabled_ = false;
+#endif
+
+ // When true this will notify |render_frame_metadata_observer_client_| of all
// frame submissions.
bool report_all_frame_submissions_for_testing_enabled_ = false;
diff --git a/content/renderer/render_frame_metadata_observer_impl_unittest.cc b/content/renderer/render_frame_metadata_observer_impl_unittest.cc
index 1f5803a..c569d77 100644
--- a/content/renderer/render_frame_metadata_observer_impl_unittest.cc
+++ b/content/renderer/render_frame_metadata_observer_impl_unittest.cc
@@ -156,6 +156,91 @@
run_loop.Run();
}
}
+
+// This test verifies that a request to send root scroll changes for
+// accessibility is respected.
+TEST_F(RenderFrameMetadataObserverImplTest, SendRootScrollsForAccessibility) {
+ const uint32_t expected_frame_token = 1337;
+ viz::CompositorFrameMetadata compositor_frame_metadata;
+ compositor_frame_metadata.send_frame_token_to_embedder = false;
+ compositor_frame_metadata.frame_token = expected_frame_token;
+ cc::RenderFrameMetadata render_frame_metadata;
+
+ observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+ &compositor_frame_metadata,
+ false /* force_send */);
+ // The first RenderFrameMetadata will always get a corresponding frame token
+ // from Viz because this is the first frame.
+ EXPECT_TRUE(compositor_frame_metadata.send_frame_token_to_embedder);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(client(), OnRenderFrameMetadataChanged(expected_frame_token,
+ render_frame_metadata))
+ .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+ run_loop.Run();
+ }
+
+ // Submit with a root scroll change and then a scroll offset at top change, we
+ // should only get one notification, as the root scroll change will not
+ // trigger one,
+ render_frame_metadata.root_scroll_offset = gfx::Vector2dF(0.0f, 100.0f);
+ observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+ &compositor_frame_metadata,
+ false /* force_send */);
+ render_frame_metadata.is_scroll_offset_at_top =
+ !render_frame_metadata.is_scroll_offset_at_top;
+ observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+ &compositor_frame_metadata,
+ false /* force_send */);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(client(), OnRenderFrameMetadataChanged(expected_frame_token,
+ render_frame_metadata))
+ .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+ run_loop.Run();
+ }
+
+ // Enable reporting for root scroll changes. This will generate one
+ // notification.
+ observer_impl().ReportAllRootScrollsForAccessibility(true);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(client(), OnRenderFrameMetadataChanged(expected_frame_token,
+ render_frame_metadata))
+ .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+ run_loop.Run();
+ }
+
+ // Now send a single root scroll change, we should get the notification.
+ render_frame_metadata.root_scroll_offset = gfx::Vector2dF(0.0f, 200.0f);
+ observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+ &compositor_frame_metadata,
+ false /* force_send */);
+ {
+ base::RunLoop run_loop;
+ // The 0u frame token indicates that the client should not expect
+ // a corresponding frame token from Viz.
+ EXPECT_CALL(client(), OnRenderFrameMetadataChanged(expected_frame_token,
+ render_frame_metadata))
+ .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+ run_loop.Run();
+ }
+
+ // Send one more message to ensure that no spurious
+ // OnRenderFrameMetadataChanged messages were generated.
+ render_frame_metadata.is_scroll_offset_at_top =
+ !render_frame_metadata.is_scroll_offset_at_top;
+ observer_impl().OnRenderFrameSubmission(render_frame_metadata,
+ &compositor_frame_metadata,
+ false /* force_send */);
+ {
+ base::RunLoop run_loop;
+ EXPECT_CALL(client(), OnRenderFrameMetadataChanged(expected_frame_token,
+ render_frame_metadata))
+ .WillOnce(InvokeClosure(run_loop.QuitClosure()));
+ run_loop.Run();
+ }
+}
#endif
// This test verifies that a request to force send metadata is respected.