Send renderer accessibility UKMs less frequently

The previous URL-keyed metrics landed in
http://crrev.com/c/2237912 fired too often. Update them
to keep track of the slowest serialization and only fire
once every several minutes, or when navigating to a new
page.

Some refactoring was needed to make this testable;
hopefully that should benefit any future tests we write
too.

Bug: 1092944
Change-Id: Icf86f0234b79f15ce27d7ccb9d45841676c1a7d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277338Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#787936}

content/public/test/render_view_test.cc

@@ -264,7 +264,8 @@ void RenderViewTest::RendererBlinkPlatformImplTestOverride::Shutdown() {
   blink_platform_impl_->Shutdown();
 }
 
-RenderViewTest::RenderViewTest(bool hook_render_frame_creation) {
+RenderViewTest::RenderViewTest(bool hook_render_frame_creation)
+    : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
   // Overrides creation of RenderFrameImpl. Subclasses may wish to do this
   // themselves and it can only be done once.
   if (hook_render_frame_creation)

content/renderer/accessibility/blink_ax_tree_source.h

@@ -38,9 +38,9 @@ class ScopedFreezeBlinkAXTreeSource {
   DISALLOW_COPY_AND_ASSIGN(ScopedFreezeBlinkAXTreeSource);
 };
 
-class BlinkAXTreeSource : public ui::AXTreeSource<blink::WebAXObject,
-                                                  AXContentNodeData,
-                                                  ui::AXTreeData> {
+class CONTENT_EXPORT BlinkAXTreeSource
+    : public ui::
+          AXTreeSource<blink::WebAXObject, AXContentNodeData, ui::AXTreeData> {
  public:
   BlinkAXTreeSource(RenderFrameImpl* render_frame, ui::AXMode mode);
   ~BlinkAXTreeSource() override;

content/renderer/accessibility/render_accessibility_impl.h

@@ -29,6 +29,10 @@
 #include "ui/accessibility/ax_tree_serializer.h"
 #include "ui/gfx/geometry/rect_f.h"
 
+namespace base {
+class ElapsedTimer;
+}  // namespace base
+
 namespace blink {
 class WebDocument;
 }  // namespace blink
@@ -114,7 +118,7 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
   ~RenderAccessibilityImpl() override;
 
   ui::AXMode GetAccessibilityMode() {
-    return tree_source_.accessibility_mode();
+    return tree_source_->accessibility_mode();
   }
 
   // RenderAccessibility implementation.
@@ -233,6 +237,16 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
   // Cancels scheduled events that are not yet in flight
   void CancelScheduledEvents();
 
+  // Sends the URL-keyed metrics for the maximum amount of time spent in
+  // SendPendingAccessibilityEvents if they meet the minimum criteria for
+  // sending.
+  void MaybeSendUKM();
+
+  // Reset all of the UKM data. This can be called after sending UKM data,
+  // or after navigating to a new page when any previous data will no
+  // longer be valid.
+  void ResetUKMData();
+
   // The RenderAccessibilityManager that owns us.
   RenderAccessibilityManager* render_accessibility_manager_;
 
@@ -255,10 +269,10 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
   std::vector<DirtyObject> dirty_objects_;
 
   // The adapter that exposes Blink's accessibility tree to AXTreeSerializer.
-  BlinkAXTreeSource tree_source_;
+  std::unique_ptr<BlinkAXTreeSource> tree_source_;
 
   // The serializer that sends accessibility messages to the browser process.
-  BlinkAXTreeSerializer serializer_;
+  std::unique_ptr<BlinkAXTreeSerializer> serializer_;
 
   using PluginAXTreeSerializer = ui::AXTreeSerializer<const ui::AXNode*,
                                                       ui::AXNodeData,
@@ -296,8 +310,23 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
   // The specified page language, or empty if unknown.
   std::string page_language_;
 
+  // The URL-keyed metrics recorder interface.
   std::unique_ptr<ukm::MojoUkmRecorder> ukm_recorder_;
 
+  // The longest amount of time spent serializing the accessibility tree
+  // in SendPendingAccessibilityEvents. This is periodically uploaded as
+  // a UKM and then reset.
+  int slowest_serialization_ms_ = 0;
+
+  // The amount of time since the last UKM upload.
+  std::unique_ptr<base::ElapsedTimer> ukm_timer_;
+
+  // The UKM Source ID that corresponds to the web page represented by
+  // slowest_serialization_ms_. We report UKM before the user navigates
+  // away, or every few minutes.
+  ukm::SourceId last_ukm_source_id_;
+  std::string last_ukm_url_;
+
   // So we can queue up tasks to be executed later.
   base::WeakPtrFactory<RenderAccessibilityImpl>
       weak_factory_for_pending_events_{this};
@@ -305,6 +334,7 @@ class CONTENT_EXPORT RenderAccessibilityImpl : public RenderAccessibility,
   friend class AXImageAnnotatorTest;
   friend class PluginActionHandlingTest;
   friend class RenderAccessibilityImplTest;
+  friend class RenderAccessibilityImplUKMTest;
 
   DISALLOW_COPY_AND_ASSIGN(RenderAccessibilityImpl);
 };

content/renderer/accessibility/render_accessibility_impl_browsertest.cc

@@ -38,6 +38,7 @@
 #include "ppapi/c/private/ppp_pdf.h"
 #include "services/image_annotation/public/cpp/image_processor.h"
 #include "services/image_annotation/public/mojom/image_annotation.mojom.h"
+#include "services/metrics/public/cpp/mojo_ukm_recorder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
@@ -885,8 +886,8 @@ class AXImageAnnotatorTest : public RenderAccessibilityImplTest {
     GetRenderAccessibilityImpl()->ax_image_annotator_ =
         std::make_unique<TestAXImageAnnotator>(GetRenderAccessibilityImpl(),
                                                mock_annotator().GetRemote());
-    GetRenderAccessibilityImpl()->tree_source_.RemoveImageAnnotator();
-    GetRenderAccessibilityImpl()->tree_source_.AddImageAnnotator(
+    GetRenderAccessibilityImpl()->tree_source_->RemoveImageAnnotator();
+    GetRenderAccessibilityImpl()->tree_source_->AddImageAnnotator(
         GetRenderAccessibilityImpl()->ax_image_annotator_.get());
   }
 
@@ -1007,4 +1008,135 @@ TEST_F(AXImageAnnotatorTest, OnImageUpdated) {
   EXPECT_EQ(3u, mock_annotator().callbacks_.size());
 }
 
+// URL-keyed metrics recorder implementation that just counts the number
+// of times it's been called.
+class MockUkmRecorder : public ukm::MojoUkmRecorder {
+ public:
+  MockUkmRecorder()
+      : ukm::MojoUkmRecorder(
+            mojo::PendingRemote<ukm::mojom::UkmRecorderInterface>()) {}
+
+  void AddEntry(ukm::mojom::UkmEntryPtr entry) override { calls_++; }
+
+  int calls() const { return calls_; }
+
+ private:
+  int calls_ = 0;
+};
+
+// Subclass of BlinkAXTreeSource that retains the functionality but
+// enables simulating a serialize operation taking an arbitrarily long
+// amount of time (using simulated time).
+class TimeDelayBlinkAXTreeSource : public BlinkAXTreeSource {
+ public:
+  TimeDelayBlinkAXTreeSource(RenderFrameImpl* rfi,
+                             ui::AXMode mode,
+                             base::test::TaskEnvironment* task_environment)
+      : BlinkAXTreeSource(rfi, mode), task_environment_(task_environment) {}
+
+  void SetTimeDelayForNextSerialize(int time_delay_ms) {
+    time_delay_ms_ = time_delay_ms;
+  }
+
+  void SerializeNode(blink::WebAXObject node,
+                     AXContentNodeData* out_data) const override {
+    BlinkAXTreeSource::SerializeNode(node, out_data);
+    if (time_delay_ms_) {
+      task_environment_->FastForwardBy(
+          base::TimeDelta::FromMilliseconds(time_delay_ms_));
+      time_delay_ms_ = 0;
+    }
+  }
+
+ private:
+  mutable int time_delay_ms_ = 0;
+  base::test::TaskEnvironment* task_environment_;
+};
+
+// Tests for URL-keyed metrics.
+class RenderAccessibilityImplUKMTest : public RenderAccessibilityImplTest {
+ public:
+  void SetUp() override {
+    RenderAccessibilityImplTest::SetUp();
+    GetRenderAccessibilityImpl()->ukm_recorder_ =
+        std::make_unique<MockUkmRecorder>();
+    GetRenderAccessibilityImpl()->tree_source_ =
+        std::make_unique<TimeDelayBlinkAXTreeSource>(
+            GetRenderAccessibilityImpl()->render_frame_,
+            GetRenderAccessibilityImpl()->GetAccessibilityMode(),
+            &task_environment_);
+    GetRenderAccessibilityImpl()->serializer_ =
+        std::make_unique<BlinkAXTreeSerializer>(
+            GetRenderAccessibilityImpl()->tree_source_.get());
+  }
+
+  void TearDown() override { RenderAccessibilityImplTest::TearDown(); }
+
+  MockUkmRecorder* ukm_recorder() {
+    return static_cast<MockUkmRecorder*>(
+        GetRenderAccessibilityImpl()->ukm_recorder_.get());
+  }
+
+  void SetTimeDelayForNextSerialize(int time_delay_ms) {
+    static_cast<TimeDelayBlinkAXTreeSource*>(
+        GetRenderAccessibilityImpl()->tree_source_.get())
+        ->SetTimeDelayForNextSerialize(time_delay_ms);
+  }
+};
+
+TEST_F(RenderAccessibilityImplUKMTest, TestFireUKMs) {
+  LoadHTMLAndRefreshAccessibilityTree(R"HTML(
+      <body>
+        <input id="text" value="Hello, World">
+      </body>
+      )HTML");
+
+  // No URL-keyed metrics should be fired initially.
+  EXPECT_EQ(0, ukm_recorder()->calls());
+
+  // No URL-keyed metrics should be fired after we send one event.
+  WebDocument document = GetMainFrame()->GetDocument();
+  WebAXObject root_obj = WebAXObject::FromWebDocument(document);
+  GetRenderAccessibilityImpl()->HandleAXEvent(
+      ui::AXEvent(root_obj.AxID(), ax::mojom::Event::kChildrenChanged));
+  SendPendingAccessibilityEvents();
+  EXPECT_EQ(0, ukm_recorder()->calls());
+
+  // No URL-keyed metrics should be fired even after an event that takes
+  // 300 ms, but we should now have something to send.
+  // This must be >= kMinSerializationTimeToSendInMS
+  SetTimeDelayForNextSerialize(300);
+  GetRenderAccessibilityImpl()->HandleAXEvent(
+      ui::AXEvent(root_obj.AxID(), ax::mojom::Event::kChildrenChanged));
+  SendPendingAccessibilityEvents();
+  EXPECT_EQ(0, ukm_recorder()->calls());
+
+  // After 1000 seconds have passed, the next time we send an event we should
+  // send URL-keyed metrics.
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(1000));
+  GetRenderAccessibilityImpl()->HandleAXEvent(
+      ui::AXEvent(root_obj.AxID(), ax::mojom::Event::kChildrenChanged));
+  SendPendingAccessibilityEvents();
+  EXPECT_EQ(1, ukm_recorder()->calls());
+
+  // Send another event that takes a long (simulated) time to serialize.
+  // This must be >= kMinSerializationTimeToSendInMS
+  SetTimeDelayForNextSerialize(200);
+  GetRenderAccessibilityImpl()->HandleAXEvent(
+      ui::AXEvent(root_obj.AxID(), ax::mojom::Event::kChildrenChanged));
+  SendPendingAccessibilityEvents();
+
+  // We shouldn't have a new call to the UKM recorder yet, not enough
+  // time has elapsed.
+  EXPECT_EQ(1, ukm_recorder()->calls());
+
+  // Navigate to a new page.
+  GetRenderAccessibilityImpl()->DidCommitProvisionalLoad(
+      ui::PAGE_TRANSITION_LINK);
+
+  // Now we should have yet another UKM recorded because of the page
+  // transition.
+  EXPECT_EQ(2, ukm_recorder()->calls());
+}
+
 }  // namespace content