Commit 4e3ce924 authored by Lowell Manners's avatar Lowell Manners Committed by Commit Bot

Revert "Do not hide the aura window of RWHVA"

This reverts commit 109a13b8.

Reason for revert: This breaks the BackforwardCache, causing the wrong
site to be shown on a back/forward navigation when the document is
cached.

TBR=alexmos@chromium.org


Original change's description:
> Do not hide the aura window of RWHVA
>
> Windows placed in inactive virtual desks will be considered
> hidden by the occlusion tracker. This currently end up in
> a call to RenderWidgetHostViewAura::Hide() which unnecessarily
> hides the aura::Window. This causes issues when reflecting that
> window in the desks mini_views.
>
> This CL keeps the aura::Window visible, while updating only
> the occlusion state.
>
> BUG=866622
> TEST=Added a new test.
>
> Change-Id: I7f71cfa072240264fea2e4934f2c7350f66a74c3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1711113
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#681459}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 866622,988477, 988999
Change-Id: I7a40d0a8695682e08776a05547a36673a40d17ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1725645
Commit-Queue: Lowell Manners <lowell@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682647}
parent 2548b93c
...@@ -334,7 +334,6 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura( ...@@ -334,7 +334,6 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(
window_(nullptr), window_(nullptr),
in_shutdown_(false), in_shutdown_(false),
in_bounds_changed_(false), in_bounds_changed_(false),
is_occluded_(true),
popup_parent_host_view_(nullptr), popup_parent_host_view_(nullptr),
popup_child_host_view_(nullptr), popup_child_host_view_(nullptr),
is_loading_(false), is_loading_(false),
...@@ -479,14 +478,12 @@ void RenderWidgetHostViewAura::Show() { ...@@ -479,14 +478,12 @@ void RenderWidgetHostViewAura::Show() {
window_->GetLocalSurfaceIdAllocation()); window_->GetLocalSurfaceIdAllocation());
} }
// See documentation of |is_occluded_|. window_->Show();
DCHECK(window_->TargetVisibility());
WasUnOccluded(); WasUnOccluded();
} }
void RenderWidgetHostViewAura::Hide() { void RenderWidgetHostViewAura::Hide() {
// See documentation of |is_occluded_|. window_->Hide();
DCHECK(window_->TargetVisibility());
WasOccluded(); WasOccluded();
} }
...@@ -630,13 +627,10 @@ void RenderWidgetHostViewAura::EnsureSurfaceSynchronizedForWebTest() { ...@@ -630,13 +627,10 @@ void RenderWidgetHostViewAura::EnsureSurfaceSynchronizedForWebTest() {
} }
bool RenderWidgetHostViewAura::IsShowing() { bool RenderWidgetHostViewAura::IsShowing() {
// See documentation of |is_occluded_|. return window_->IsVisible();
DCHECK(window_->TargetVisibility());
return !is_occluded_;
} }
void RenderWidgetHostViewAura::WasUnOccluded() { void RenderWidgetHostViewAura::WasUnOccluded() {
is_occluded_ = false;
if (!host_->is_hidden()) if (!host_->is_hidden())
return; return;
...@@ -669,7 +663,6 @@ void RenderWidgetHostViewAura::WasUnOccluded() { ...@@ -669,7 +663,6 @@ void RenderWidgetHostViewAura::WasUnOccluded() {
} }
void RenderWidgetHostViewAura::WasOccluded() { void RenderWidgetHostViewAura::WasOccluded() {
is_occluded_ = true;
if (!host()->is_hidden()) { if (!host()->is_hidden()) {
host()->WasHidden(); host()->WasHidden();
if (delegated_frame_host_) if (delegated_frame_host_)
...@@ -1699,8 +1692,6 @@ void RenderWidgetHostViewAura::OnWindowDestroyed(aura::Window* window) { ...@@ -1699,8 +1692,6 @@ void RenderWidgetHostViewAura::OnWindowDestroyed(aura::Window* window) {
} }
void RenderWidgetHostViewAura::OnWindowTargetVisibilityChanged(bool visible) { void RenderWidgetHostViewAura::OnWindowTargetVisibilityChanged(bool visible) {
// See documentation of |is_occluded_|.
DCHECK(visible);
} }
bool RenderWidgetHostViewAura::HasHitTestMask() const { bool RenderWidgetHostViewAura::HasHitTestMask() const {
...@@ -2033,10 +2024,6 @@ void RenderWidgetHostViewAura::CreateAuraWindow(aura::client::WindowType type) { ...@@ -2033,10 +2024,6 @@ void RenderWidgetHostViewAura::CreateAuraWindow(aura::client::WindowType type) {
// Init(), because it needs to have the layer. // Init(), because it needs to have the layer.
if (frame_sink_id_.is_valid()) if (frame_sink_id_.is_valid())
window_->SetEmbedFrameSinkId(frame_sink_id_); window_->SetEmbedFrameSinkId(frame_sink_id_);
// The |window_|'s visiblility remains visible the entire time.
// See documentation of |is_occluded_|.
window_->Show();
} }
void RenderWidgetHostViewAura::CreateDelegatedFrameHostClient() { void RenderWidgetHostViewAura::CreateDelegatedFrameHostClient() {
......
...@@ -593,14 +593,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAura ...@@ -593,14 +593,6 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
// True if in the process of handling a window bounds changed notification. // True if in the process of handling a window bounds changed notification.
bool in_bounds_changed_; bool in_bounds_changed_;
// True if the content of this view is occluded (i.e. not visible).
// Note that we keep |window_|'s target visibility true all the time. Its
// actual visibility (i.e. Window::IsVisible()) will depend on its location in
// the window tree hierarchy and whether its layer is drawn or not (due to
// e.g. occlusion). Hiding |window_| is unnecessary and will cause issues when
// the browser or app is mirrored for alt-tab, or virtual desks mini_views.
bool is_occluded_;
// Our parent host view, if this is a popup. NULL otherwise. // Our parent host view, if this is a popup. NULL otherwise.
RenderWidgetHostViewAura* popup_parent_host_view_; RenderWidgetHostViewAura* popup_parent_host_view_;
......
...@@ -72,10 +72,6 @@ class RenderWidgetHostViewAuraBrowserTest : public ContentBrowserTest { ...@@ -72,10 +72,6 @@ class RenderWidgetHostViewAuraBrowserTest : public ContentBrowserTest {
GetRenderViewHost()->GetWidget()->GetView()); GetRenderViewHost()->GetWidget()->GetView());
} }
aura::Window* GetWindow() const {
return GetRenderWidgetHostView()->window();
}
DelegatedFrameHost* GetDelegatedFrameHost() const { DelegatedFrameHost* GetDelegatedFrameHost() const {
return GetRenderWidgetHostView()->delegated_frame_host_.get(); return GetRenderWidgetHostView()->delegated_frame_host_.get();
} }
...@@ -104,7 +100,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest, ...@@ -104,7 +100,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
// Hide the view and evict the frame. This should trigger a copy of the stale // Hide the view and evict the frame. This should trigger a copy of the stale
// frame content. // frame content.
GetRenderWidgetHostView()->Hide(); GetRenderWidgetHostView()->Hide();
EXPECT_TRUE(GetWindow()->TargetVisibility());
static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost()) static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost())
->EvictDelegatedFrame(); ->EvictDelegatedFrame();
EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_, EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_,
...@@ -120,7 +115,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest, ...@@ -120,7 +115,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
// Unhidding the view should reset the stale content layer to show the new // Unhidding the view should reset the stale content layer to show the new
// frame content. // frame content.
GetRenderWidgetHostView()->Show(); GetRenderWidgetHostView()->Show();
EXPECT_TRUE(GetWindow()->TargetVisibility());
EXPECT_FALSE( EXPECT_FALSE(
GetDelegatedFrameHost()->stale_content_layer_->has_external_content()); GetDelegatedFrameHost()->stale_content_layer_->has_external_content());
} }
...@@ -146,14 +140,12 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest, ...@@ -146,14 +140,12 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
// Hide the view and evict the frame. This should trigger a copy of the stale // Hide the view and evict the frame. This should trigger a copy of the stale
// frame content. // frame content.
GetRenderWidgetHostView()->Hide(); GetRenderWidgetHostView()->Hide();
EXPECT_TRUE(GetWindow()->TargetVisibility());
static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost()) static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost())
->EvictDelegatedFrame(); ->EvictDelegatedFrame();
EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_, EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_,
DelegatedFrameHost::FrameEvictionState::kPendingEvictionRequests); DelegatedFrameHost::FrameEvictionState::kPendingEvictionRequests);
GetRenderWidgetHostView()->Show(); GetRenderWidgetHostView()->Show();
EXPECT_TRUE(GetWindow()->TargetVisibility());
EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_, EXPECT_EQ(GetDelegatedFrameHost()->frame_eviction_state_,
DelegatedFrameHost::FrameEvictionState::kNotStarted); DelegatedFrameHost::FrameEvictionState::kNotStarted);
...@@ -188,7 +180,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest, ...@@ -188,7 +180,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
// Hide the view and evict the frame. This should not trigger a copy of the // Hide the view and evict the frame. This should not trigger a copy of the
// stale frame content as the WebContentDelegate returns false. // stale frame content as the WebContentDelegate returns false.
GetRenderWidgetHostView()->Hide(); GetRenderWidgetHostView()->Hide();
EXPECT_TRUE(GetWindow()->TargetVisibility());
static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost()) static_cast<viz::FrameEvictorClient*>(GetDelegatedFrameHost())
->EvictDelegatedFrame(); ->EvictDelegatedFrame();
...@@ -201,25 +192,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest, ...@@ -201,25 +192,6 @@ IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
EXPECT_FALSE( EXPECT_FALSE(
GetDelegatedFrameHost()->stale_content_layer_->has_external_content()); GetDelegatedFrameHost()->stale_content_layer_->has_external_content());
} }
IN_PROC_BROWSER_TEST_F(RenderWidgetHostViewAuraBrowserTest,
OcclusionDoesntAffectWindowTargetVisibility) {
NavigateToURL(shell(), GURL(kMinimalPageDataURL));
EXPECT_TRUE(GetWindow()->TargetVisibility());
EXPECT_TRUE(GetRenderWidgetHostView()->IsShowing());
// Hide the window's parent, the content should be occluded but the window's
// target visibility should remain visible.
GetWindow()->parent()->Hide();
EXPECT_TRUE(GetWindow()->TargetVisibility());
EXPECT_FALSE(GetRenderWidgetHostView()->IsShowing());
// Show the parent again, and expect that the contents are unoccluded again.
GetWindow()->parent()->Show();
EXPECT_TRUE(GetWindow()->TargetVisibility());
EXPECT_TRUE(GetRenderWidgetHostView()->IsShowing());
}
#endif // #if defined(OS_CHROMEOS) #endif // #if defined(OS_CHROMEOS)
} // namespace content } // namespace content
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