Commit 06293526 authored by Sven Zheng's avatar Sven Zheng Committed by Commit Bot

DialogBrowserTest support pixel tests

With this change, all DialogBrowserTest should be abled to verify pixel correctness.
To use them as pixel test, add flag --browser-ui-tests-verify-pixels.
TEST: browser_tests.exe --gtest_filter=PermissionDialogTest.*
--browser-ui-tests-verify-pixels --enable-pixel-output-in-tests
--build-revision=b6aaf42b --no-luci-auth
I've approved the following images so these tests can pass now.
https://chrome-gpu-gold.skia.org/detail?test=BrowserUiDialog_PermissionDialogTest_InvokeUi_protocol_handlers&digest=877f6df6101e926f24c5b9bc3cf111b3
https://chrome-gpu-gold.skia.org/detail?test=BrowserUiDialog_PermissionDialogTest_InvokeUi_camera&digest=cf09792802b243e0a33bf4d45e2e8047
I didn't start with test_browser_ui because most of the tests are inherited from test_browser_dialog.

Next step:
1 Add some tests to fyi bots to monitor the stability.
2 Fix some tests that don't have stable UI. Eg disable animation for
https://chrome-gpu-gold.skia.org/detail?test=BrowserUiDialog_AuthenticatorDialogViewTest_InvokeUi_default&digest=2442c011949e9f98f4442f066bb90e55

Bug: 958242
Change-Id: I7bc048b792a5e303e8ee948fda228fa664395ca1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756604
Commit-Queue: Sven Zheng <svenzheng@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688211}
parent 2bbd2b7e
......@@ -10,6 +10,7 @@
#include "base/stl_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "chrome/test/pixel/browser_skia_gold_pixel_diff.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_CHROMEOS)
......@@ -21,6 +22,7 @@
#endif
#if defined(TOOLKIT_VIEWS)
#include "base/strings/strcat.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
#include "ui/views/test/widget_test.h"
......@@ -53,17 +55,43 @@ class WidgetCloser {
DISALLOW_COPY_AND_ASSIGN(WidgetCloser);
};
class CompositorObserverEnd : public ui::CompositorObserver {
public:
void OnCompositingEnded(ui::Compositor* compositor) override {
base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
run_loop_.QuitClosure());
}
void WaitForComplete() { run_loop_.Run(); }
private:
base::RunLoop run_loop_;
};
#endif // defined(TOOLKIT_VIEWS)
} // namespace
TestBrowserDialog::TestBrowserDialog() = default;
TestBrowserDialog::TestBrowserDialog() : TestBrowserUi() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
"browser-ui-tests-verify-pixels")) {
pixel_diff_ = std::make_unique<BrowserSkiaGoldPixelDiff>();
}
}
TestBrowserDialog::~TestBrowserDialog() = default;
void TestBrowserDialog::PreShow() {
UpdateWidgets();
}
void CompareAsync(const BrowserSkiaGoldPixelDiff* pixel_diff,
const std::string& name,
const views::View* view,
bool* result) {
*result = pixel_diff->CompareScreenshot(name, view);
}
// This returns true if exactly one views widget was shown that is a dialog or
// has a name matching the test-specified name, and if that window is in the
// work area (if |should_verify_dialog_bounds_| is true).
......@@ -87,13 +115,41 @@ bool TestBrowserDialog::VerifyUi() {
if (added.size() != 1)
return false;
views::Widget* dialog_widget = *(added.begin());
// TODO(https://crbug.com/958242) support Mac for pixel tests.
#if !defined(OS_MACOSX)
if (pixel_diff_) {
// Wait for painting complete.
auto* compositor = dialog_widget->GetNativeView()->layer()->GetCompositor();
CompositorObserverEnd obend;
compositor->AddObserver(&obend);
obend.WaitForComplete();
compositor->RemoveObserver(&obend);
base::ScopedAllowBlockingForTesting allow_blocking;
pixel_diff_->Init(dialog_widget, "BrowserUiDialog");
auto* test_info = testing::UnitTest::GetInstance()->current_test_info();
const std::string test_name =
base::StrCat({test_info->test_case_name(), "_", test_info->name()});
bool result = false;
base::RunLoop run_loop;
base::ThreadTaskRunnerHandle::Get()->PostTaskAndReply(
FROM_HERE,
base::BindOnce(CompareAsync, pixel_diff_.get(), test_name,
dialog_widget->GetContentsView(), &result),
run_loop.QuitClosure());
run_loop.Run();
if (!result)
return false;
}
#endif // OS_MACOSX
if (!should_verify_dialog_bounds_)
return true;
// Verify that the dialog's dimensions do not exceed the display's work area
// bounds, which may be smaller than its bounds(), e.g. in the case of the
// docked magnifier or Chromevox being enabled.
views::Widget* dialog_widget = *(added.begin());
const gfx::Rect dialog_bounds = dialog_widget->GetWindowBoundsInScreen();
gfx::NativeWindow native_window = dialog_widget->GetNativeWindow();
DCHECK(native_window);
......
......@@ -13,6 +13,8 @@
#include "ui/views/widget/widget.h"
#endif
class BrowserSkiaGoldPixelDiff;
// A dialog-specific subclass of TestBrowserUi, which will verify that a test
// showed a single dialog.
class TestBrowserDialog : public TestBrowserUi {
......@@ -55,6 +57,9 @@ class TestBrowserDialog : public TestBrowserUi {
// This should always be true, but some dialogs don't yet size themselves
// properly. https://crbug.com/893292.
bool should_verify_dialog_bounds_ = true;
// If this variable is set, VerifyUi will verify pixel correctness for
// the dialog.
std::unique_ptr<BrowserSkiaGoldPixelDiff> pixel_diff_;
DISALLOW_COPY_AND_ASSIGN(TestBrowserDialog);
};
......
......@@ -25,7 +25,7 @@ class Image;
class BrowserSkiaGoldPixelDiff : public SkiaGoldPixelDiff {
public:
BrowserSkiaGoldPixelDiff();
~BrowserSkiaGoldPixelDiff();
~BrowserSkiaGoldPixelDiff() override;
// Call Init method before using this class.
// Args:
// widget The instance you plan to take screenshots with.
......
......@@ -19,7 +19,7 @@ class SkBitmap;
class SkiaGoldPixelDiff {
public:
SkiaGoldPixelDiff();
~SkiaGoldPixelDiff();
virtual ~SkiaGoldPixelDiff();
// Call Init method before using this class.
// Args:
// screenshot_prefix The prefix for your screenshot name on GCS.
......
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