Commit c588f9f7 authored by Edwin Joe's avatar Edwin Joe Committed by Commit Bot

Fix the Browser.Tabs.TotalSwitchDuration UMA metric for MacOS

In MacOS, tab switch follows a different path than in Windows/Linux.
It calls DelegatedFrameHost::WasShown when we are inside the call stack
of NativeViewHostMac::AttachNativeView. This results in the check
whether we have saved frames or not inside RWHVMac::WasUnOccluded
to always return true since we have called DelegatedFrameHost::WasShown
previously.

This CL saves the saved frame condition before we call the first
DelegatedFrameHost::WasShown in MacOS, and use this value to determine
whether we have saved frames or not at a tab switch.

Note that this might cause an upward spike in the
MPArch.RWH_TabSwitchPaintDuration UMA metric since previously,
it always record time as if we have saved frames, i.e. the
presentation time for the delegated frame host. This CL will
change that behavior to record time properly based on whether
we have saved frames or not, which leads to a possible spike
in the metric.

Bug: 921120
Change-Id: If196665bb287a8b3e27ce24f4151ff88ae5f841f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1538560
Commit-Queue: Edwin Joe <ejoe@google.com>
Reviewed-by: default avatarJonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644429}
parent dcf7a316
...@@ -109,6 +109,10 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient, ...@@ -109,6 +109,10 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
viz::FrameSinkId GetRootFrameSinkId(); viz::FrameSinkId GetRootFrameSinkId();
bool has_saved_frame_before_state_transition() const {
return has_saved_frame_before_state_transition_;
}
const gfx::Size& GetRendererSize() const { return dfh_size_dip_; } const gfx::Size& GetRendererSize() const { return dfh_size_dip_; }
void GetRendererScreenInfo(ScreenInfo* screen_info) const; void GetRendererScreenInfo(ScreenInfo* screen_info) const;
viz::ScopedSurfaceIdAllocator GetScopedRendererSurfaceIdAllocator( viz::ScopedSurfaceIdAllocator GetScopedRendererSurfaceIdAllocator(
...@@ -193,6 +197,16 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient, ...@@ -193,6 +197,16 @@ class CONTENT_EXPORT BrowserCompositorMac : public DelegatedFrameHostClient,
gfx::Size dfh_size_dip_; gfx::Size dfh_size_dip_;
display::Display dfh_display_; display::Display dfh_display_;
// This is used to cache the saved frame state to be used for tab switching
// metric. In tab switch in MacOS, DelegatedFrameHost::WasShown is called once
// inside BrowserCompositor::TransitionToState before it is called again by
// RenderWidgetHostViewMac::WasUnOccluded. Since tab switching metric begins
// inside RenderWidgetHostView(Mac|Aura), DelegatedFrameHost::HasSavedFrame
// will always return true in Mac when we check later.
// TODO(jonross): unify the order of DelegatedFrameHost::WasShown and
// RenderWidgetHostViewBase::WadUnOccluded across platforms.
bool has_saved_frame_before_state_transition_ = false;
bool is_first_navigation_ = true; bool is_first_navigation_ = true;
base::WeakPtrFactory<BrowserCompositorMac> weak_factory_; base::WeakPtrFactory<BrowserCompositorMac> weak_factory_;
......
...@@ -291,6 +291,8 @@ void BrowserCompositorMac::TransitionToState(State new_state) { ...@@ -291,6 +291,8 @@ void BrowserCompositorMac::TransitionToState(State new_state) {
delegated_frame_host_->AttachToCompositor(GetCompositor()); delegated_frame_host_->AttachToCompositor(GetCompositor());
if (!dfh_local_surface_id_allocator_.HasValidLocalSurfaceIdAllocation()) if (!dfh_local_surface_id_allocator_.HasValidLocalSurfaceIdAllocation())
dfh_local_surface_id_allocator_.GenerateId(); dfh_local_surface_id_allocator_.GenerateId();
has_saved_frame_before_state_transition_ =
delegated_frame_host_->HasSavedFrame();
delegated_frame_host_->WasShown( delegated_frame_host_->WasShown(
GetRendererLocalSurfaceIdAllocation().local_surface_id(), dfh_size_dip_, GetRendererLocalSurfaceIdAllocation().local_surface_id(), dfh_size_dip_,
false /* record_presentation_time */); false /* record_presentation_time */);
......
...@@ -469,7 +469,7 @@ void RenderWidgetHostViewMac::WasUnOccluded() { ...@@ -469,7 +469,7 @@ void RenderWidgetHostViewMac::WasUnOccluded() {
browser_compositor_->GetDelegatedFrameHost(); browser_compositor_->GetDelegatedFrameHost();
bool has_saved_frame = bool has_saved_frame =
delegated_frame_host ? delegated_frame_host->HasSavedFrame() : false; browser_compositor_->has_saved_frame_before_state_transition();
auto tab_switch_start_time = GetAndResetLastTabChangeStartTime(); auto tab_switch_start_time = GetAndResetLastTabChangeStartTime();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment