Hide fullscreen rotation jank

content/browser/renderer_host/render_widget_host_view_android.h

 // 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.
 
 #ifndef CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_VIEW_ANDROID_H_
 #define CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_VIEW_ANDROID_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 143 matching lines @@
   void ClearCompositorFrame() override;
   void SetIsInVR(bool is_in_vr) override;
   bool IsInVR() const override;
   void DidOverscroll(const ui::DidOverscrollParams& params) override;
   void DidStopFlinging() override;
   void ShowDisambiguationPopup(const gfx::Rect& rect_pixels,
                                const SkBitmap& zoomed_bitmap) override;
   std::unique_ptr<SyntheticGestureTarget> CreateSyntheticGestureTarget()
       override;
   void OnDidNavigateMainFrameToNewPage() override;
+  void OnFullscreenStateChanged(bool entered_fullscreen) override;
   void SetNeedsBeginFrames(bool needs_begin_frames) override;
   cc::FrameSinkId GetFrameSinkId() override;
   cc::FrameSinkId FrameSinkIdAtPoint(cc::SurfaceHittestDelegate* delegate,
                                      const gfx::Point& point,
                                      gfx::Point* transformed_point) override;
   void ProcessMouseEvent(const blink::WebMouseEvent& event,
                          const ui::LatencyInfo& latency) override;
   void ProcessMouseWheelEvent(const blink::WebMouseWheelEvent& event,
                               const ui::LatencyInfo& latency) override;
   void ProcessTouchEvent(const blink::WebTouchEvent& event,
@@ skipping 57 matching lines @@
   void ReclaimResources(const cc::ReturnedResourceArray& resources) override;
 
   // cc::BeginFrameObserver implementation.
   void OnBeginFrame(const cc::BeginFrameArgs& args) override;
   const cc::BeginFrameArgs& LastUsedBeginFrameArgs() const override;
   void OnBeginFrameSourcePausedChanged(bool paused) override;
 
   // Non-virtual methods
   void SetContentViewCore(ContentViewCoreImpl* content_view_core);
   SkColor GetCachedBackgroundColor() const;
+  void ClearThumbnailPlaceholder();
   void SendKeyEvent(const NativeWebKeyboardEvent& event);
   void SendMouseEvent(const ui::MotionEventAndroid&, int action_button);
   void SendMouseWheelEvent(const blink::WebMouseWheelEvent& event);
   void SendGestureEvent(const blink::WebGestureEvent& event);
   void ResolveTapDisambiguation(double timestamp_seconds,
                                 gfx::Point tap_viewport_offset,
                                 bool is_long_press);
   void set_ime_adapter(ImeAdapterAndroid* ime_adapter) {
     ime_adapter_android_ = ime_adapter;
   }
@@ skipping 46 matching lines @@
                               RenderWidgetHostViewBase* updated_view) override;
   void OnTextSelectionChanged(TextInputManager* text_input_manager,
                               RenderWidgetHostViewBase* updated_view) override;
 
   ImeAdapterAndroid* ime_adapter_for_testing() { return ime_adapter_android_; }
 
   // Exposed for tests.
   cc::SurfaceId SurfaceIdForTesting() const override;
 
  private:
+  enum class FullscreenState {
+    kNotFullscreen,
+    kEnteringFullscreen,
+    kFullscreen,
+    kExitingFullscreen,
+    kFullscreenRotation,
+  };
+
   void RunAckCallbacks();
 
   void SendReclaimCompositorResources(bool is_swap_ack);
 
   void OnFrameMetadataUpdated(const cc::CompositorFrameMetadata& frame_metadata,
                               bool is_transparent);
 
   void ShowInternal();
   void HideInternal();
   void AttachLayers();
@@ skipping 25 matching lines @@
   void RequestDisallowInterceptTouchEvent();
 
   bool SyncCompositorOnMessageReceived(const IPC::Message& message);
 
   void ComputeEventLatencyOSTouchHistograms(const ui::MotionEvent& event);
 
   void CreateOverscrollControllerIfPossible();
 
   WebContentsAccessibilityAndroid* GetWebContentsAccessibilityAndroid() const;
 
+  void UpdateFullscreenStateAndEvictFrameIfNeeded();
+  void UpdateFullscreenState();
+  bool IsInFullscreenTransition() const;
+
   // The model object.
   RenderWidgetHostImpl* host_;
 
   // The begin frame source being observed.  Null if none.
   cc::BeginFrameSource* begin_frame_source_;
   cc::BeginFrameArgs last_begin_frame_args_;
   uint32_t latest_confirmed_begin_frame_source_id_;
   uint64_t latest_confirmed_begin_frame_sequence_number_;
 
   // Indicates whether and for what reason a request for begin frames has been
@@ skipping 59 matching lines @@
   std::unique_ptr<viz::FrameEvictor> frame_evictor_;
 
   bool observing_root_window_;
 
   // The last scroll offset of the view.
   gfx::Vector2dF last_scroll_offset_;
 
   float prev_top_shown_pix_;
   float prev_bottom_shown_pix_;
 
+  FullscreenState fullscreen_state_ = FullscreenState::kNotFullscreen;
+
+  // While we could simply take a mismatch in fullscreen states as the trigger
+  // for a transition, and assume the transition to have finished once the
+  // fullscreen states and sizes match, we have cases where we reach this
+  // consistent state before the physical backing size changes. Therefore we
+  // defer assuming the transition to be completed until a resize event is
+  // received, which is marked by |fullscreen_transition_awaiting_resize_|.
+  //
+  // Note that this assumes that every transition will be followed by a change
+  // in the view's physical backing size.
+  bool fullscreen_transition_awaiting_resize_ = false;

[aelias_OOO_until_Jul13, 2017/06/22 02:52:06]

Ping again when you make progress on removing this and/or is_fullscreen, which
are the hard open question.

We don't have an exact idea how best to fix it and the only thing me and boliu@
agree on is that they should and somehow can be done more cleanly.  I suggest
trying out different options out of the menu we've offered until you find one
that works reliably (it's hard for me to know just by theorycrafting about it). 
Let us know if it would help to have a short VC about it if you're stuck or
wondering if a given solution would be approved of (afternoon times work better
for me).

[steimel, 2017/06/29 00:38:51]

According to Khushal, the discussion on crbug.com/736092 has led to the
conclusion that we should keep both the fullscreen metadata and resizing checks.
Let me know if I've misinterpreted

[aelias_OOO_until_Jul13, 2017/06/29 01:05:18]

The initial concerns expressed by me and Bo didn't have to do with Viz.  The
discussion on crbug.com/736092 concluded that there are no additional
constraints related to Viz, but it didn't clear them up.

There's a complexity problem in that you're combining awaiting_resize,
is_fullscreen, and frame size to arrive at a decision.  Basically you have three
low-signals you're combining to arrive at a high-quality one.  That means this
code depends on subtleties of many different faraway subsystems all at once --
it is brittle and hard to understand, and the contract that outside code is
supposed to abide by in the future to interact with it is not clearly defined. 
There has to be a simpler way to achieve your goal.  Please improve the quality
(i.e. timing) of one or more of the signals at its source to remove at least one
of the three.

+  bool current_frame_is_fullscreen_ = false;
+  bool web_contents_is_fullscreen_ = false;
+
   cc::mojom::MojoCompositorFrameSinkClient* renderer_compositor_frame_sink_ =
       nullptr;
 
   base::ObserverList<DestructionObserver> destruction_observers_;
 
   MouseWheelPhaseHandler mouse_wheel_phase_handler_;
 
   base::WeakPtrFactory<RenderWidgetHostViewAndroid> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewAndroid);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_VIEW_ANDROID_H_