Reland again "Start gpu channel and compositor mojo pipe collection eagerly"
[email protected]
TBR=piman
SHERIFFS: This changes timing in the renderer, if (probably ChromeVOX)
tests get flaky they should be disabled please instead of reverting
this.
The flaky tests causing the last iteration of revert were disabled
in https://crrev.com/c/1361585.
This reverts commit d38eb8e01e09dc05802b062bf159f86942fce09e.
Original change's description:
> Start gpu channel and compositor mojo pipe collection eagerly
>
> When a RenderWidget is frozen, its compositor is stopped. However when
> the main frame is being navigated, we want to start requesting mojo
> pipes for the gpu and display compositor immediately so that it can
> happen in parallel with the navigation loading.
>
> Previously, we just always did this when creating a RenderWidget, and
> left them active when freezing the RenderWidget. After 678f025f7cde75
> however, we don't, and this causes time-to-first-pixels regressions
> on navigation.
>
> So we add a WarmupCompositor() method to RenderWidget, and call this
> from RenderFrameImpl::CreateFrame() when it makes the provisional
> frame, and the WebFrameWidget, since we expect to make use of the
> main frame RenderWidget shortly.
>
> Then, if RenderFrameImpl::FrameDetached() occurs, due to the
> navigation failing, we will AbortWarmupCompositor() to drop the
> mojo pipes instead of holding onto them indefinitely.
>
> This recovers the loading regressions introduced, while also not
> allocating mojo channels for frozen RenderWidgets indefinitely.
>
> In order to do this reasonably, we drop the "callback" from the
> request to RenderThreadImpl::RequestNewLayerTreeFrameSink so that
> it always returns something immediately. This way RenderWidget
> does not need to worry about having a task run to collect the
> new frame sink, and ordering with tasks from the compositor to
> collect it.
>
> [email protected]
>
> Change-Id: I6007e26e1622006652ce3619cdd080539d11e7b0
> Bug: 905191
> Reviewed-on: https://chromium-review.googlesource.com/c/1341073
> Commit-Queue: danakj <[email protected]>
> Reviewed-by: danakj <[email protected]>
> Reviewed-by: Daniel Cheng <[email protected]>
> Reviewed-by: Antoine Labour <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#610909}
Change-Id: I100c65758b673a27f7d90bdbd6369abf8e8c608f
Reviewed-on: https://chromium-review.googlesource.com/c/1361598
Reviewed-by: danakj <[email protected]>
Commit-Queue: danakj <[email protected]>
Cr-Commit-Position: refs/heads/master@{#613670}diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h
index f0c6fd9..c5e16c64 100644
--- a/content/renderer/render_widget.h
+++ b/content/renderer/render_widget.h
@@ -240,6 +240,20 @@
void SetIsFrozen(bool is_frozen);
bool is_frozen() const { return is_frozen_; }
+ // When a RenderWidget is created, even if frozen, if we expect to unfreeze
+ // and use the RenderWidget imminently, then we want to pre-emptively start
+ // the process of getting the resources needed for the compositor. This helps
+ // to parallelize the critical path to first pixels with the loading process.
+ // This should only be called when the RenderWidget is frozen, otherwise it
+ // would be redundant at best. Non-frozen RenderWidgets will start to warmup
+ // immediately on their own.
+ void WarmupCompositor();
+ // If after calling WarmupCompositor() we can determine that the RenderWidget
+ // does not expect to be used shortly after all, call this to cancel the
+ // warmup process and release any unused resources that had been created by
+ // it.
+ void AbortWarmupCompositor();
+
// This is true once a Close IPC has been received. The actual action of
// closing must be done on another stack frame, in case the IPC receipt
// is in a nested message loop and will unwind back up to javascript (from
@@ -754,6 +768,15 @@
// belongs to the frame tree associated with this RenderWidget.
blink::WebLocalFrame* GetFocusedWebLocalFrameInWidget() const;
+ // Called with the resulting frame sink from WarmupCompositor() since frame
+ // sink creation can be asynchronous.
+ void OnReplyForWarmupCompositor(std::unique_ptr<cc::LayerTreeFrameSink> sink);
+
+ // Common code shared to execute the creation of a LayerTreeFrameSink, shared
+ // by the warmup and standard request paths. Callers should verify they really
+ // want to do this before calling it as this method does no verification.
+ void DoRequestNewLayerTreeFrameSink(LayerTreeFrameSinkCallback callback);
+
// Routing ID that allows us to communicate to the parent browser process
// RenderWidgetHost.
const int32_t routing_id_;
@@ -927,6 +950,18 @@
// Wraps the |webwidget_| as a MouseLockDispatcher::LockTarget interface.
std::unique_ptr<MouseLockDispatcher::LockTarget> webwidget_mouse_lock_target_;
+ // Set to true while a warmup is in progress. Set to false if the warmup is
+ // completed or aborted. If aborted, the reply callback is also cancelled by
+ // invalidating the |warmup_weak_ptr_factory_|.
+ bool warmup_frame_sink_request_pending_ = false;
+ // Set after warmup completes without being aborted. This frame sink will be
+ // returned on the next request for a frame sink instead of creating a new
+ // one.
+ std::unique_ptr<cc::LayerTreeFrameSink> warmup_frame_sink_;
+ // Set if a request for a frame sink arrives while a warmup is in progress.
+ // Then this stores the request to be satisfied once the warmup completes.
+ LayerTreeFrameSinkCallback after_warmup_callback_;
+
viz::LocalSurfaceIdAllocation local_surface_id_allocation_from_parent_;
// Indicates whether this widget has focus.
@@ -996,6 +1031,10 @@
uint32_t last_capture_sequence_number_ = 0u;
+ // Used to generate a callback for the reply when making the warmup frame
+ // sink, and to cancel that callback if the warmup is aborted.
+ base::WeakPtrFactory<RenderWidget> warmup_weak_ptr_factory_;
+
base::WeakPtrFactory<RenderWidget> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(RenderWidget);
