Commit bd896631 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Keep frame overlays topmost

This fixes a regression caused by crrev.com/710881. The reason is that
before that CL, we had an extra level in GraphicsLayer tree (which was
the root GraphicsLayer created by VisualViewport), and the frame
overlays were added in that level after the root layer of the contents.
With the CL, frame overlays and the top-level contents GraphicsLayers
started to have the same parent, so the frame overlays may be behind
the contents layers after later compositing updates. The problem
affected "persistent" frame overlays such as the DevTools
overlay which lives when DevTools is running.

Now in FrameOverlay::UpdatePrePaint(), always keep the GraphicsLayer the
last child of the root layer to ensure it's the topmost. This will
slightly impact performance when there are multiple frame overlays
which is rare.

The next step will be to refactor FrameOverlay not to create
GraphicsLayers, and to combine code paths for pre-CompositeAfterPaint
and CompositeAfterPaint.

Bug: 1057071
Change-Id: I0fed6e69d4a90b38b7a636712f4349e0985ff100
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135891Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756413}
parent 163db598
......@@ -75,7 +75,9 @@ void FrameOverlay::UpdatePrePaint() {
}
DCHECK(parent_layer);
if (layer_->Parent() != parent_layer)
if (layer_->Parent() != parent_layer ||
// Keep the layer the last child of parent to make it topmost.
parent_layer->Children().back() != layer_.get())
parent_layer->AddChild(layer_.get());
layer_->SetLayerState(DefaultPropertyTreeState(), IntPoint());
layer_->SetSize(gfx::Size(Size()));
......
......@@ -14,8 +14,10 @@
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/frame/visual_viewport.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/platform/graphics/color.h"
#include "third_party/blink/renderer/platform/graphics/graphics_context.h"
#include "third_party/blink/renderer/platform/graphics/graphics_layer.h"
#include "third_party/blink/renderer/platform/graphics/paint/drawing_recorder.h"
#include "third_party/blink/renderer/platform/graphics/paint/paint_controller.h"
#include "third_party/blink/renderer/platform/graphics/paint/paint_controller_test.h"
......@@ -162,6 +164,40 @@ TEST_P(FrameOverlayTest, DeviceEmulationScale) {
}
}
TEST_P(FrameOverlayTest, LayerOrder) {
// This test doesn't apply in CompositeAfterPaint.
if (RuntimeEnabledFeatures::CompositeAfterPaintEnabled())
return;
auto frame_overlay1 = CreateSolidYellowOverlay();
auto frame_overlay2 = CreateSolidYellowOverlay();
frame_overlay1->UpdatePrePaint();
frame_overlay2->UpdatePrePaint();
auto* parent_layer = GetWebView()
->MainFrameImpl()
->GetFrameView()
->GetLayoutView()
->Compositor()
->PaintRootGraphicsLayer();
ASSERT_EQ(3u, parent_layer->Children().size());
EXPECT_EQ(parent_layer, frame_overlay1->GetGraphicsLayer()->Parent());
EXPECT_EQ(parent_layer->Children()[1], frame_overlay1->GetGraphicsLayer());
EXPECT_EQ(parent_layer, frame_overlay2->GetGraphicsLayer()->Parent());
EXPECT_EQ(parent_layer->Children()[2], frame_overlay2->GetGraphicsLayer());
auto extra_layer = std::make_unique<GraphicsLayer>(parent_layer->Client());
parent_layer->AddChild(extra_layer.get());
frame_overlay1->UpdatePrePaint();
frame_overlay2->UpdatePrePaint();
ASSERT_EQ(4u, parent_layer->Children().size());
EXPECT_EQ(parent_layer, frame_overlay1->GetGraphicsLayer()->Parent());
EXPECT_EQ(parent_layer->Children()[2], frame_overlay1->GetGraphicsLayer());
EXPECT_EQ(parent_layer, frame_overlay2->GetGraphicsLayer()->Parent());
EXPECT_EQ(parent_layer->Children()[3], frame_overlay2->GetGraphicsLayer());
}
TEST_P(FrameOverlayTest, VisualRect) {
std::unique_ptr<FrameOverlay> frame_overlay = CreateSolidYellowOverlay();
frame_overlay->UpdatePrePaint();
......
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