[android] Make RWHVAndroid a BeginFrameObserver.

content/browser/renderer_host/render_widget_host_view_android.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
 
 #include <android/bitmap.h>
 
 #include <utility>
 
@@ skipping 59 matching lines @@
 #include "content/public/common/content_switches.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "gpu/config/gpu_driver_bug_workaround_type.h"
 #include "ipc/ipc_message_macros.h"
 #include "ipc/ipc_message_start.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
 #include "third_party/skia/include/core/SkCanvas.h"
-#include "ui/android/delegated_frame_host_android.h"
 #include "ui/android/window_android.h"
 #include "ui/android/window_android_compositor.h"
 #include "ui/base/layout.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/events/blink/blink_event_util.h"
 #include "ui/events/blink/did_overscroll_params.h"
 #include "ui/events/blink/web_input_event_traits.h"
 #include "ui/events/gesture_detection/gesture_provider_config_helper.h"
@@ skipping 334 matching lines @@
       static_cast<RenderWidgetHostViewAndroid*>(widget->GetView())
           ->OnLostResources();
     }
   }
 }
 
 RenderWidgetHostViewAndroid::RenderWidgetHostViewAndroid(
     RenderWidgetHostImpl* widget_host,
     ContentViewCoreImpl* content_view_core)
     : host_(widget_host),
-      outstanding_vsync_requests_(0),
+      begin_frame_source_(nullptr),
+      outstanding_begin_frame_requests_(0),
       is_showing_(!widget_host->is_hidden()),
       is_window_visible_(true),
       is_window_activity_started_(true),
       is_showing_overscroll_glow_(true),
       content_view_core_(nullptr),
       ime_adapter_android_(this),
       cached_background_color_(SK_ColorWHITE),
       last_compositor_frame_sink_id_(kUndefinedCompositorFrameSinkId),
       gesture_provider_(ui::GetGestureProviderConfig(
                             ui::GestureProviderConfigType::CURRENT_PLATFORM),
                         this),
       stylus_text_selector_(this),
       using_browser_compositor_(CompositorImpl::IsInitialized()),
       synchronous_compositor_client_(nullptr),
       frame_evictor_(new DelegatedFrameEvictor(this)),
       locks_on_frame_count_(0),
       observing_root_window_(false),
       weak_ptr_factory_(this) {
   // Set the layer which will hold the content layer for this view. The content
   // layer is managed by the DelegatedFrameHost.
   view_.SetLayer(cc::Layer::Create());
   if (using_browser_compositor_) {
     delegated_frame_host_.reset(new ui::DelegatedFrameHostAndroid(
-        &view_, cached_background_color_,
-        base::Bind(&RenderWidgetHostViewAndroid::ReturnResources,
-                   weak_ptr_factory_.GetWeakPtr())));
+        &view_, cached_background_color_, this));
   }
 
   host_->SetView(this);
   SetContentViewCore(content_view_core);
 
   CreateOverscrollControllerIfPossible();
 
   if (GetTextInputManager())
     GetTextInputManager()->AddObserver(this);
 }
@@ skipping 319 matching lines @@
     delegated_frame_host_->UpdateBackgroundColor(color);
 
   if (content_view_core_)
     content_view_core_->OnBackgroundColorChanged(color);
 }
 
 void RenderWidgetHostViewAndroid::SetNeedsBeginFrames(bool needs_begin_frames) {
   TRACE_EVENT1("cc", "RenderWidgetHostViewAndroid::SetNeedsBeginFrames",
                "needs_begin_frames", needs_begin_frames);
   if (needs_begin_frames)
-    RequestVSyncUpdate(PERSISTENT_BEGIN_FRAME);
+    AddBeginFrameRequest(PERSISTENT_BEGIN_FRAME);
   else
-    outstanding_vsync_requests_ &= ~PERSISTENT_BEGIN_FRAME;
+    ClearBeginFrameRequest(PERSISTENT_BEGIN_FRAME);
 }
 
 void RenderWidgetHostViewAndroid::OnStartContentIntent(
     const GURL& content_url, bool is_main_frame) {
   if (content_view_core_)
     content_view_core_->StartContentIntent(content_url, is_main_frame);
 }
 
 void RenderWidgetHostViewAndroid::OnSmartClipDataExtracted(
     const base::string16& text,
@@ skipping 33 matching lines @@
       event, result.moved_beyond_slop_region);
   ui::LatencyInfo latency_info(ui::SourceEventType::TOUCH);
   latency_info.AddLatencyNumber(ui::INPUT_EVENT_LATENCY_UI_COMPONENT, 0, 0);
   host_->ForwardTouchEventWithLatencyInfo(web_event, latency_info);
 
   // Send a proactive BeginFrame for this vsync to reduce scroll latency for
   // scroll-inducing touch events. Note that Android's Choreographer ensures
   // that BeginFrame requests made during ACTION_MOVE dispatch will be honored
   // in the same vsync phase.
   if (observing_root_window_ && result.moved_beyond_slop_region)
-    RequestVSyncUpdate(BEGIN_FRAME);
+    AddBeginFrameRequest(BEGIN_FRAME);
 
   return true;
 }
 
 bool RenderWidgetHostViewAndroid::OnTouchHandleEvent(
     const ui::MotionEvent& event) {
   return selection_controller_ &&
          selection_controller_->WillHandleTouchEvent(event);
 }
 
@@ skipping 510 matching lines @@
 
   view_.GetLayer()->SetHideLayerAndSubtree(false);
 
   frame_evictor_->SetVisible(true);
 
   if (overscroll_controller_)
     overscroll_controller_->Enable();
 
   host_->WasShown(ui::LatencyInfo());
 
-  if (content_view_core_) {
+  if (content_view_core_ && view_.GetWindowAndroid()) {
     StartObservingRootWindow();
-    RequestVSyncUpdate(BEGIN_FRAME);
+    AddBeginFrameRequest(BEGIN_FRAME);
   }
 }
 
 void RenderWidgetHostViewAndroid::HideInternal() {
   DCHECK(!is_showing_ || !is_window_activity_started_ || !is_window_visible_)
       << "Hide called when the widget should be shown.";
 
   // Only preserve the frontbuffer if the activity was stopped while the
   // window is still visible. This avoids visual artificts when transitioning
   // between activities.
@@ skipping 22 matching lines @@
   if (overscroll_controller_)
     overscroll_controller_->Disable();
 
   RunAckCallbacks();
 
   // Inform the renderer that we are being hidden so it can reduce its resource
   // utilization.
   host_->WasHidden();
 }
 
-void RenderWidgetHostViewAndroid::RequestVSyncUpdate(uint32_t requests) {
-  bool should_request_vsync = !outstanding_vsync_requests_ && requests;
-  outstanding_vsync_requests_ |= requests;
+void RenderWidgetHostViewAndroid::SetBeginFrameSource(
+    cc::BeginFrameSource* begin_frame_source) {
+  if (begin_frame_source_ == begin_frame_source)
+    return;
 
-  // Note that if we're not currently observing the root window, outstanding
-  // vsync requests will be pushed if/when we resume observing in
-  // |StartObservingRootWindow()|.
-  if (observing_root_window_ && should_request_vsync) {
-    ui::WindowAndroid* windowAndroid = view_.GetWindowAndroid();
-    DCHECK(windowAndroid);
-    // TODO(boliu): This check should be redundant with
-    // |observing_root_window_| check above. However we are receiving trickle
-    // of crash reports (crbug.com/639868) with no root cause. Should
-    // investigate more when time allows what corner case is missed.
-    if (windowAndroid)
-      windowAndroid->RequestVSyncUpdate();
-  }
+  if (begin_frame_source_ && outstanding_begin_frame_requests_)
+    begin_frame_source_->RemoveObserver(this);
+  begin_frame_source_ = begin_frame_source;
+  if (begin_frame_source_ && outstanding_begin_frame_requests_)
+    begin_frame_source_->AddObserver(this);
+}
+
+void RenderWidgetHostViewAndroid::AddBeginFrameRequest(
+    BeginFrameRequestType request) {
+  uint32_t prior_requests = outstanding_begin_frame_requests_;
+  outstanding_begin_frame_requests_ = prior_requests | request;
+
+  // Note that if we don't currently have a BeginFrameSource, outstanding begin
+  // frame requests will be pushed if/when we get one during
+  // |StartObservingRootWindow()| or when the DelegatedFrameHostAndroid sets it.
+  cc::BeginFrameSource* source = begin_frame_source_;
+  if (source && outstanding_begin_frame_requests_ && !prior_requests)
+    source->AddObserver(this);
+}
+
+void RenderWidgetHostViewAndroid::ClearBeginFrameRequest(
+    BeginFrameRequestType request) {
+  uint32_t prior_requests = outstanding_begin_frame_requests_;
+  outstanding_begin_frame_requests_ = prior_requests & ~request;
+
+  cc::BeginFrameSource* source = begin_frame_source_;
+  if (source && !outstanding_begin_frame_requests_ && prior_requests)
+    source->RemoveObserver(this);
 }
 
 void RenderWidgetHostViewAndroid::StartObservingRootWindow() {
   DCHECK(content_view_core_);
-  // TODO(yusufo): This will need to have a better fallback for cases where
-  // setContentViewCore is called with a valid ContentViewCore without a window.
   DCHECK(view_.GetWindowAndroid());
   DCHECK(is_showing_);
   if (observing_root_window_)
     return;
 
   observing_root_window_ = true;
   if (host_)
     host_->Send(new ViewMsg_SetBeginFramePaused(host_->GetRoutingID(), false));
   view_.GetWindowAndroid()->AddObserver(this);
-
-  // Clear existing vsync requests to allow a request to the new window.
-  uint32_t outstanding_vsync_requests = outstanding_vsync_requests_;
-  outstanding_vsync_requests_ = 0;
-  RequestVSyncUpdate(outstanding_vsync_requests);
+  // When using browser compositor, DelegatedFrameHostAndroid provides the BFS.
+  if (!using_browser_compositor_)
+    SetBeginFrameSource(view_.GetWindowAndroid()->GetBeginFrameSource());
 
   ui::WindowAndroidCompositor* compositor =
       view_.GetWindowAndroid()->GetCompositor();
   if (compositor) {
     delegated_frame_host_->RegisterFrameSinkHierarchy(
         compositor->GetFrameSinkId());
   }
 }
 
 void RenderWidgetHostViewAndroid::StopObservingRootWindow() {
   if (!(view_.GetWindowAndroid())) {
     DCHECK(!observing_root_window_);
     return;
   }
 
   if (!observing_root_window_)
     return;
 
   // Reset window state variables to their defaults.
   is_window_activity_started_ = true;
   is_window_visible_ = true;
   observing_root_window_ = false;
   if (host_)
     host_->Send(new ViewMsg_SetBeginFramePaused(host_->GetRoutingID(), true));
   view_.GetWindowAndroid()->RemoveObserver(this);
+  if (!using_browser_compositor_)
+    SetBeginFrameSource(nullptr);
   // If the DFH has already been destroyed, it will have cleaned itself up.
   // This happens in some WebView cases.
   if (delegated_frame_host_)
     delegated_frame_host_->UnregisterFrameSinkHierarchy();
+  DCHECK(!begin_frame_source_);
 }
 
-void RenderWidgetHostViewAndroid::SendBeginFrame(base::TimeTicks frame_time,
-                                                 base::TimeDelta vsync_period) {
+void RenderWidgetHostViewAndroid::SendBeginFrame(cc::BeginFrameArgs args) {
   TRACE_EVENT1("cc", "RenderWidgetHostViewAndroid::SendBeginFrame",
-               "frame_time_us", frame_time.ToInternalValue());
+               "frame_time_us", args.frame_time.ToInternalValue());
 
   // Synchronous compositor does not use deadline-based scheduling.
   // TODO(brianderson): Replace this hardcoded deadline after Android
-  // switches to Surfaces and the Browser's commit isn't in the critcal path.
-  base::TimeTicks deadline =
-      sync_compositor_ ? base::TimeTicks() : frame_time + (vsync_period * 0.6);
-  host_->Send(new ViewMsg_BeginFrame(
-      host_->GetRoutingID(),
-      cc::BeginFrameArgs::Create(BEGINFRAME_FROM_HERE, frame_time, deadline,
-                                 vsync_period, cc::BeginFrameArgs::NORMAL)));
+  // switches to Surfaces and the Browser's commit isn't in the critical path.
+  args.deadline = sync_compositor_ ? base::TimeTicks()
+  : args.frame_time + (args.interval * 0.6);
+  host_->Send(new ViewMsg_BeginFrame(host_->GetRoutingID(), args));
   if (sync_compositor_)
     sync_compositor_->DidSendBeginFrame(view_.GetWindowAndroid());
 }
 
 bool RenderWidgetHostViewAndroid::Animate(base::TimeTicks frame_time) {
   bool needs_animate = false;
   if (overscroll_controller_ && is_showing_overscroll_glow_) {
     needs_animate |= overscroll_controller_->Animate(
         frame_time, content_view_core_->GetViewAndroid()->GetLayer());
   }
@@ skipping 93 matching lines @@
     if (shim && gpu_data &&
         gpu_data->IsDriverBugWorkaroundActive(gpu::WAKE_UP_GPU_BEFORE_DRAWING))
       shim->Send(new GpuMsg_WakeUpGpu);
   }
 
   return INPUT_EVENT_ACK_STATE_NOT_CONSUMED;
 }
 
 void RenderWidgetHostViewAndroid::OnSetNeedsFlushInput() {
   TRACE_EVENT0("input", "RenderWidgetHostViewAndroid::OnSetNeedsFlushInput");
-  RequestVSyncUpdate(FLUSH_INPUT);
+  AddBeginFrameRequest(FLUSH_INPUT);
 }
 
 BrowserAccessibilityManager*
     RenderWidgetHostViewAndroid::CreateBrowserAccessibilityManager(
         BrowserAccessibilityDelegate* delegate, bool for_root_frame) {
   base::android::ScopedJavaLocalRef<jobject> content_view_core_obj;
   if (for_root_frame && host_ && content_view_core_)
     content_view_core_obj = content_view_core_->GetJavaObject();
   return new BrowserAccessibilityManagerAndroid(
       content_view_core_obj,
@@ skipping 161 matching lines @@
     if (content_view_core_)
       obj = content_view_core_->GetJavaObject();
     manager->ToBrowserAccessibilityManagerAndroid()->SetContentViewCore(obj);
   }
 
   if (!content_view_core_) {
     sync_compositor_.reset();
     return;
   }
 
-  if (is_showing_)
+  if (is_showing_ && view_.GetWindowAndroid())
     StartObservingRootWindow();
 
   if (resize)
     WasResized();
 
   if (!selection_controller_)
     selection_controller_ = CreateSelectionController(this, content_view_core_);
 
   if (content_view_core_)
     CreateOverscrollControllerIfPossible();
@@ skipping 55 matching lines @@
 }
 
 void RenderWidgetHostViewAndroid::OnDetachedFromWindow() {
   StopObservingRootWindow();
   OnDetachCompositor();
 }
 
 void RenderWidgetHostViewAndroid::OnAttachCompositor() {
   DCHECK(content_view_core_);
   CreateOverscrollControllerIfPossible();
-  ui::WindowAndroidCompositor* compositor =
-      view_.GetWindowAndroid()->GetCompositor();
-  delegated_frame_host_->RegisterFrameSinkHierarchy(
-      compositor->GetFrameSinkId());
+  if (observing_root_window_) {
+    ui::WindowAndroidCompositor* compositor =
+        view_.GetWindowAndroid()->GetCompositor();
+    delegated_frame_host_->RegisterFrameSinkHierarchy(
+        compositor->GetFrameSinkId());
+  }
 }
 
 void RenderWidgetHostViewAndroid::OnDetachCompositor() {
   DCHECK(content_view_core_);
   DCHECK(using_browser_compositor_);
   RunAckCallbacks();
   overscroll_controller_.reset();
   delegated_frame_host_->UnregisterFrameSinkHierarchy();
 }
 
-void RenderWidgetHostViewAndroid::OnVSync(base::TimeTicks frame_time,
-                                          base::TimeDelta vsync_period) {
-  TRACE_EVENT0("cc,benchmark", "RenderWidgetHostViewAndroid::OnVSync");
+void RenderWidgetHostViewAndroid::OnBeginFrame(const cc::BeginFrameArgs& args) {
+  TRACE_EVENT0("cc,benchmark", "RenderWidgetHostViewAndroid::OnBeginFrame");
   if (!host_)
     return;
 
-  if (outstanding_vsync_requests_ & FLUSH_INPUT) {
-    outstanding_vsync_requests_ &= ~FLUSH_INPUT;
+  // In sync mode, we disregard missed frame args to ensure that
+  // SynchronousCompositorBrowserFilter::SyncStateAfterVSync will be called
+  // during WindowAndroid::WindowBeginFrameSource::OnVSync() observer iteration.
+  if (sync_compositor_ && args.type == cc::BeginFrameArgs::MISSED)
+    return;
+
+  // crbug.com/675289: Update |last_begin_frame_args_| before handling
+  // |outstanding_begin_frame_requests_| to prevent the BeginFrameSource from
+  // sending the same MISSED args in an infinite loop. This may otherwise happen
+  // if |host_->FlushInput()| causes a synchronous OnSetNeedsFlushInput(), which
+  // can lead to |begin_frame_source_->AddObserver()| and OnBeginFrame().

[boliu, 2016/12/20 16:13:50]

how does moving last_begin_frame_args_ up fix this exactly? That the FlushInput
gets the right args?


instead of the complicated thing in window_android, how about this fix:
Only call Add/RemoveObserver at the *end* of this method. This way, it would be
impossible to toggle back and forth between states

[Eric Seckler, 2016/12/20 16:35:07]

It prevents the BeginFrameSource from sending us the same args again: During
AddObserver(), it checks if our last_begin_frame_args_ are older than its latest
ones. If they are older, it sends us a missed BeginFrame.

The reason we were getting stuck is that we called (RemoveObserver() and then)
AddObserver() before updating our last_begin_frame_args_, indicating to the
BeginFrameSource that we did not yet receive/handle its latest BeginFrameArgs.
I have two reservations against this and would rather like to keep the check in
WindowAndroid. Note though that this is a different issue only tangentially
related to crbug.com/675289, which I discovered during debugging. (Sorry, should
have made this clear I guess :))

The problem there is that we might send the same BeginFrameArgs to an observer
twice during OnVSync(), which is forbidden as noted in the comment of
BeginFrameObserver::OnBeginFrame().

The RWHVA is only one BeginFrameObserver, so changing it to not exhibit the
forbidden behavior doesn't really solve it, I think. There's no contract
forbidding other observers to remove/add themselves during iteration.
(WindowBeginFrameSource is exposed widely through the compositor.)

Regarding RWHVA: If possible, I'd like to avoid moving Add/RemoveObserver out of
the Add/ClearBeginFrameRequest methods - since otherwise we'd have to check if
we need to AddObserver() in all callsites of AddBeginFrameRequest().

[boliu, 2016/12/20 16:56:09]

oh, then do say that in the comment, that updating last begin frame args will
prevent infinite recursion. also don't need the bug number I guess, since it
will be fixed
how about actually comparing LastBeginFrameArgs then, rather than keeping the
set?

But then probably should implement LastUsedBeginFrameArgs properly in
synchronous_compositor_browser_filter

+  last_begin_frame_args_ = args;
+
+  if (outstanding_begin_frame_requests_ & FLUSH_INPUT) {
+    ClearBeginFrameRequest(FLUSH_INPUT);
     host_->FlushInput();
   }
 
-  if (outstanding_vsync_requests_ & BEGIN_FRAME ||
-      outstanding_vsync_requests_ & PERSISTENT_BEGIN_FRAME) {
-    outstanding_vsync_requests_ &= ~BEGIN_FRAME;
-    SendBeginFrame(frame_time, vsync_period);
+  if ((outstanding_begin_frame_requests_ & BEGIN_FRAME) ||
+      (outstanding_begin_frame_requests_ & PERSISTENT_BEGIN_FRAME)) {
+    ClearBeginFrameRequest(BEGIN_FRAME);
+    SendBeginFrame(args);
   }
+}
 
-  // This allows for SendBeginFrame and FlushInput to modify
-  // outstanding_vsync_requests.
-  uint32_t outstanding_vsync_requests = outstanding_vsync_requests_;
-  outstanding_vsync_requests_ = 0;
-  RequestVSyncUpdate(outstanding_vsync_requests);
+const cc::BeginFrameArgs& RenderWidgetHostViewAndroid::LastUsedBeginFrameArgs()
+    const {
+  return last_begin_frame_args_;
+}
+
+void RenderWidgetHostViewAndroid::OnBeginFrameSourcePausedChanged(bool paused) {
+  // The BeginFrameSources we listen to don't use this. For WebView, we signal
+  // the "paused" state to the RenderWidget when our window attaches/detaches,
+  // see |StartObservingRootWindow()| and |StopObservingRootWindow()|.
+  DCHECK(!paused);
 }
 
 void RenderWidgetHostViewAndroid::OnAnimate(base::TimeTicks begin_frame_time) {
   if (Animate(begin_frame_time))
     SetNeedsAnimate();
 }
 
 void RenderWidgetHostViewAndroid::OnActivityStopped() {
   TRACE_EVENT0("browser", "RenderWidgetHostViewAndroid::OnActivityStopped");
   DCHECK(observing_root_window_);
@@ skipping 101 matching lines @@
   if (!compositor)
     return;
 
   overscroll_controller_ = base::MakeUnique<OverscrollControllerAndroid>(
       overscroll_refresh_handler, compositor,
       ui::GetScaleFactorForNativeView(GetNativeView()));
   is_showing_overscroll_glow_ = true;
 }
 
 }  // namespace content