Commit 48614c8b authored by David Bokan's avatar David Bokan Committed by Commit Bot

Ensure all tests run with a valid ScrollbarTheme

In https://crrev.com/c/2119933 I added a new call to GetScrollbarTheme
which caused a number of unit tests to start crashing.

On Android, unit tests run without a ThemeEngine
(Platform::Current()->ThemeEngine() == nullptr). This means that they
must use a mock scrollbar theme, otherwise anything scrollbar
related might crash. Currently, it was possible for some tests to avoid
setting a mock theme and run successfully if they got lucky and avoided
code paths that called into the theme.

This CL adds a DCHECK when a blink::Page is created to ensure we can
access the theme. This flushed out all tests that were in this situation
and we fix them here. It also ensures future tests are made to install a
mock theme so we don't run into this situation again.

Bug: 1039501
Change-Id: I78dbf406b6168a71b67e75f09ef824b5cdc2a8d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125952Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Reviewed-by: default avatarDale Curtis <dalecurtis@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754800}
parent 0d1fd241
......@@ -66,6 +66,7 @@
#include "third_party/blink/public/platform/web_url_response.h"
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/public/web/web_local_frame_client.h"
#include "third_party/blink/public/web/web_testing_support.h"
#include "third_party/blink/public/web/web_view.h"
#include "third_party/blink/public/web/web_widget.h"
#include "url/gurl.h"
......@@ -311,7 +312,9 @@ class MockVideoFrameCompositor : public VideoFrameCompositor {
void(const viz::SurfaceId&, base::TimeTicks, media::VideoRotation, bool));
};
class WebMediaPlayerImplTest : public testing::Test {
class WebMediaPlayerImplTest
: public testing::Test,
private blink::WebTestingSupport::WebScopedMockScrollbars {
public:
WebMediaPlayerImplTest()
: media_thread_("MediaThreadForTest"),
......
......@@ -32,11 +32,26 @@
namespace blink {
class ScopedMockOverlayScrollbars;
class WebTestingSupport {
public:
static void InjectInternalsObject(WebLocalFrame*);
static void ResetInternalsObject(WebLocalFrame*);
static void InjectInternalsObject(v8::Local<v8::Context>);
// Use this to install a mock scrollbar theme for tests. To use, simply
// inherit your test class from this or instantiate it manually. The
// constructor and destructor will enable and disable the mock theme. For
// tests within Blink, use ScopedMockOverlayScrollbars instead.
class WebScopedMockScrollbars {
public:
WebScopedMockScrollbars();
~WebScopedMockScrollbars();
private:
std::unique_ptr<ScopedMockOverlayScrollbars> use_mock_scrollbars_;
};
};
}
......
......@@ -12,6 +12,7 @@
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
#include "third_party/blink/renderer/core/testing/page_test_base.h"
#include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h"
#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
#include "third_party/blink/renderer/platform/testing/testing_platform_support_with_mock_scheduler.h"
......@@ -40,7 +41,8 @@ int NetworkActivityCheckerForTest::GetActiveConnections() {
return active_connections_;
}
class InteractiveDetectorTest : public testing::Test {
class InteractiveDetectorTest : public testing::Test,
public ScopedMockOverlayScrollbars {
public:
InteractiveDetectorTest() {
platform_->AdvanceClockSeconds(1);
......
......@@ -213,6 +213,13 @@ Page::Page(PageClients& page_clients)
web_text_autosizer_page_info_({0, 0, 1.f}) {
DCHECK(!AllPages().Contains(this));
AllPages().insert(this);
// Try to dereference the scrollbar theme. This is here to ensure tests are
// correctly setting up their platform theme or mocking scrollbars. On
// Android, unit tests run without a ThemeEngine and thus must set a mock
// ScrollbarTheme, if they don't this call will crash. To set a mock theme,
// see ScopedMockOverlayScrollbars or WebScopedMockScrollbars.
DCHECK(&GetScrollbarTheme());
}
Page::~Page() {
......
......@@ -27,11 +27,18 @@
#include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_internals_partial.h"
#include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
#include "third_party/blink/renderer/core/testing/v8/web_core_test_support.h"
#include "v8/include/v8.h"
namespace blink {
WebTestingSupport::WebScopedMockScrollbars::WebScopedMockScrollbars()
: use_mock_scrollbars_(new ScopedMockOverlayScrollbars) {}
WebTestingSupport::WebScopedMockScrollbars::~WebScopedMockScrollbars() =
default;
void WebTestingSupport::InjectInternalsObject(WebLocalFrame* frame) {
V8InternalsPartial::Initialize();
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
......
......@@ -41,6 +41,7 @@
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/dom/events/event_queue.h"
#include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_database.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_database_callbacks.h"
#include "third_party/blink/renderer/modules/indexeddb/idb_key.h"
......@@ -73,7 +74,8 @@ class FakeIDBDatabaseCallbacks final : public IDBDatabaseCallbacks {
void OnComplete(int64_t transaction_id) override {}
};
class IDBTransactionTest : public testing::Test {
class IDBTransactionTest : public testing::Test,
public ScopedMockOverlayScrollbars {
protected:
void SetUp() override {
url_loader_mock_factory_ = platform_->GetURLLoaderMockFactory();
......
......@@ -19,6 +19,7 @@
#include "third_party/blink/public/platform/scheduler/test/renderer_scheduler_test_support.h"
#include "third_party/blink/public/web/web_heap.h"
#include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_testing.h"
#include "third_party/blink/renderer/core/testing/scoped_mock_overlay_scrollbars.h"
#include "third_party/blink/renderer/modules/mediarecorder/fake_encoded_video_frame.h"
#include "third_party/blink/renderer/modules/mediarecorder/media_recorder.h"
#include "third_party/blink/renderer/modules/mediarecorder/media_recorder_handler.h"
......@@ -107,7 +108,8 @@ class MockMediaRecorder : public MediaRecorder {
MOCK_METHOD1(OnError, void(const String& message));
};
class MediaRecorderHandlerTest : public TestWithParam<MediaRecorderTestParams> {
class MediaRecorderHandlerTest : public TestWithParam<MediaRecorderTestParams>,
public ScopedMockOverlayScrollbars {
public:
MediaRecorderHandlerTest()
: media_recorder_handler_(MakeGarbageCollected<MediaRecorderHandler>(
......@@ -561,7 +563,8 @@ static const MediaRecorderPassthroughTestParams
};
class MediaRecorderHandlerPassthroughTest
: public TestWithParam<MediaRecorderPassthroughTestParams> {
: public TestWithParam<MediaRecorderPassthroughTestParams>,
public ScopedMockOverlayScrollbars {
public:
MediaRecorderHandlerPassthroughTest() {
registry_.Init();
......
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