aw: Prevent reentering OnBeginFrame

This CL modifies BeginFrameSourceWebView::BeginFrameObserver to prevent
reentrancy of OnBeginFrame. If the clients of this BeginFrameSource add
or remove observers during OnBeginFrame this will cause the source to
subscribe/unsubscribe from upstream BeginFrameSource and potentially
get missed BeginFrame. To avoid this we save last_used_begin_frames_
before actually processing BeginFrame.

Bug: 1044340
Change-Id: I6df71952c0efa1e8bf7db4953453684b955c42e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2013808Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734160}

android_webview/browser/gfx/begin_frame_source_webview.cc

@@ -11,23 +11,28 @@
 namespace android_webview {
 
 class BeginFrameSourceWebView::BeginFrameObserver
-    : public viz::BeginFrameObserverBase {
+    : public viz::BeginFrameObserver {
  public:
-  BeginFrameObserver(BeginFrameSourceWebView* owner) : owner_(owner) {
-    wants_animate_only_begin_frames_ = true;
-  }
+  BeginFrameObserver(BeginFrameSourceWebView* owner) : owner_(owner) {}
 
-  bool OnBeginFrameDerivedImpl(const viz::BeginFrameArgs& args) override {
+  void OnBeginFrame(const viz::BeginFrameArgs& args) override {
+    last_used_begin_frame_args_ = args;
     owner_->SendBeginFrame(args);
-    return true;
+  }
+
+  const viz::BeginFrameArgs& LastUsedBeginFrameArgs() const override {
+    return last_used_begin_frame_args_;
   }
 
   void OnBeginFrameSourcePausedChanged(bool paused) override {
     owner_->OnSetBeginFrameSourcePaused(paused);
   }
 
+  bool WantsAnimateOnlyBeginFrames() const override { return true; }
+
  private:
   BeginFrameSourceWebView* const owner_;
+  viz::BeginFrameArgs last_used_begin_frame_args_;
 };
 
 BeginFrameSourceWebView::BeginFrameSourceClient::BeginFrameSourceClient(

android_webview/browser/gfx/begin_frame_source_webview_unittest.cc

@@ -199,4 +199,22 @@ TEST_F(BeginFrameSourceWebViewTest, ChildPauseChangeOnSetParent) {
   begin_frame_source_.RemoveObserver(&observer_);
 }
 
+TEST_F(BeginFrameSourceWebViewTest, Reentrancy) {
+  begin_frame_source_.SetParentSource(&root_begin_frame_source_);
+  begin_frame_source_.AddObserver(&observer_);
+
+  // Re-Add observer inside OnBeginFrame so it will trigger missed BeginFrame
+  EXPECT_CALL(observer_, OnBeginFrame(testing::_))
+      .WillRepeatedly(testing::Invoke([&](const viz::BeginFrameArgs& args) {
+        if (args.type == viz::BeginFrameArgs::MISSED)
+          return;
+        begin_frame_source_.RemoveObserver(&observer_);
+        begin_frame_source_.AddObserver(&observer_);
+      }));
+
+  test_begin_frame_source_.OnBeginFrame(BeginFrameArgsForTesting(1));
+
+  begin_frame_source_.RemoveObserver(&observer_);
+}
+
 }  // namespace android_webview