Commit c955c4cd authored by Yuki Shiino's avatar Yuki Shiino Committed by Commit Bot

Revert "Move ownership of SpellCheckClient to TestRunnerBindings"

This reverts commit 2c5f2493.

Reason for revert: Suspicious about the test failures:
https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20Leak/16356


Original change's description:
> Move ownership of SpellCheckClient to TestRunnerBindings
> 
> There is one per frame, and WebFrameTestProxy doesn't actually need
> access to it. So simplify the ownership/pointer graph by moving it
> entirely inside TestRunnerBindings.
> 
> R=​avi@chromium.org
> 
> Bug: 866140
> Change-Id: I091c1aa899c70db0ac1bafabf19707ea3245e88e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2281284
> Commit-Queue: Avi Drissman <avi@chromium.org>
> Auto-Submit: danakj <danakj@chromium.org>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#785236}

TBR=avi@chromium.org,danakj@chromium.org

Change-Id: I64bcfec65efea9650d509cffc5f54b75c2bc1d63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 866140
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2280975Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785271}
parent 3460ebab
#include "base/debug/stack_trace.h"
// Copyright 2014 The Chromium Authors. All rights reserved. // Copyright 2014 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
...@@ -174,6 +175,7 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> { ...@@ -174,6 +175,7 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
static void Install(TestRunner* test_runner, static void Install(TestRunner* test_runner,
WebFrameTestProxy* frame, WebFrameTestProxy* frame,
SpellCheckClient* spell_check,
bool is_wpt_reftest, bool is_wpt_reftest,
bool is_frame_part_of_main_test_window); bool is_frame_part_of_main_test_window);
...@@ -211,7 +213,8 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> { ...@@ -211,7 +213,8 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
}; };
explicit TestRunnerBindings(TestRunner* test_runner, explicit TestRunnerBindings(TestRunner* test_runner,
WebFrameTestProxy* frame); WebFrameTestProxy* frame,
SpellCheckClient* spell_check);
~TestRunnerBindings() override; ~TestRunnerBindings() override;
// gin::Wrappable overrides. // gin::Wrappable overrides.
...@@ -397,7 +400,7 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> { ...@@ -397,7 +400,7 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
TestRunner* runner_; TestRunner* runner_;
WebFrameTestProxy* const frame_; WebFrameTestProxy* const frame_;
SpellCheckClient spell_check_; SpellCheckClient* const spell_check_;
TestPreferences prefs_; TestPreferences prefs_;
std::unique_ptr<AppBannerService> app_banner_service_; std::unique_ptr<AppBannerService> app_banner_service_;
...@@ -411,6 +414,7 @@ gin::WrapperInfo TestRunnerBindings::kWrapperInfo = {gin::kEmbedderNativeGin}; ...@@ -411,6 +414,7 @@ gin::WrapperInfo TestRunnerBindings::kWrapperInfo = {gin::kEmbedderNativeGin};
// static // static
void TestRunnerBindings::Install(TestRunner* test_runner, void TestRunnerBindings::Install(TestRunner* test_runner,
WebFrameTestProxy* frame, WebFrameTestProxy* frame,
SpellCheckClient* spell_check,
bool is_wpt_test, bool is_wpt_test,
bool is_frame_part_of_main_test_window) { bool is_frame_part_of_main_test_window) {
v8::Isolate* isolate = blink::MainThreadIsolate(); v8::Isolate* isolate = blink::MainThreadIsolate();
...@@ -421,7 +425,8 @@ void TestRunnerBindings::Install(TestRunner* test_runner, ...@@ -421,7 +425,8 @@ void TestRunnerBindings::Install(TestRunner* test_runner,
v8::Context::Scope context_scope(context); v8::Context::Scope context_scope(context);
TestRunnerBindings* wrapped = new TestRunnerBindings(test_runner, frame); TestRunnerBindings* wrapped =
new TestRunnerBindings(test_runner, frame, spell_check);
gin::Handle<TestRunnerBindings> bindings = gin::Handle<TestRunnerBindings> bindings =
gin::CreateHandle(isolate, wrapped); gin::CreateHandle(isolate, wrapped);
CHECK(!bindings.IsEmpty()); CHECK(!bindings.IsEmpty());
...@@ -484,13 +489,12 @@ void TestRunnerBindings::Install(TestRunner* test_runner, ...@@ -484,13 +489,12 @@ void TestRunnerBindings::Install(TestRunner* test_runner,
} }
TestRunnerBindings::TestRunnerBindings(TestRunner* runner, TestRunnerBindings::TestRunnerBindings(TestRunner* runner,
WebFrameTestProxy* frame) WebFrameTestProxy* frame,
SpellCheckClient* spell_check)
: frame_observer_(this, frame), : frame_observer_(this, frame),
runner_(runner), runner_(runner),
frame_(frame), frame_(frame),
spell_check_(frame->GetWebFrame()) { spell_check_(spell_check) {}
frame->GetWebFrame()->SetTextCheckClient(&spell_check_);
}
TestRunnerBindings::~TestRunnerBindings() = default; TestRunnerBindings::~TestRunnerBindings() = default;
...@@ -1021,20 +1025,20 @@ void TestRunnerBindings::SetFilePathForMockFileDialog( ...@@ -1021,20 +1025,20 @@ void TestRunnerBindings::SetFilePathForMockFileDialog(
void TestRunnerBindings::SetMockSpellCheckerEnabled(bool enabled) { void TestRunnerBindings::SetMockSpellCheckerEnabled(bool enabled) {
if (invalid_) if (invalid_)
return; return;
spell_check_.SetEnabled(enabled); spell_check_->SetEnabled(enabled);
} }
void TestRunnerBindings::SetSpellCheckResolvedCallback( void TestRunnerBindings::SetSpellCheckResolvedCallback(
v8::Local<v8::Function> callback) { v8::Local<v8::Function> callback) {
if (invalid_) if (invalid_)
return; return;
spell_check_.SetSpellCheckResolvedCallback(callback); spell_check_->SetSpellCheckResolvedCallback(callback);
} }
void TestRunnerBindings::RemoveSpellCheckResolvedCallback() { void TestRunnerBindings::RemoveSpellCheckResolvedCallback() {
if (invalid_) if (invalid_)
return; return;
spell_check_.RemoveSpellCheckResolvedCallback(); spell_check_->RemoveSpellCheckResolvedCallback();
} }
v8::Local<v8::Value> v8::Local<v8::Value>
...@@ -2198,10 +2202,11 @@ TestRunner::TestRunner(TestInterfaces* interfaces) ...@@ -2198,10 +2202,11 @@ TestRunner::TestRunner(TestInterfaces* interfaces)
TestRunner::~TestRunner() = default; TestRunner::~TestRunner() = default;
void TestRunner::Install(WebFrameTestProxy* frame) { void TestRunner::Install(WebFrameTestProxy* frame,
SpellCheckClient* spell_check) {
// In WPT, only reftests generate pixel results. // In WPT, only reftests generate pixel results.
TestRunnerBindings::Install( TestRunnerBindings::Install(
this, frame, IsWebPlatformTestsMode(), this, frame, spell_check, IsWebPlatformTestsMode(),
IsFramePartOfMainTestWindow(frame->GetWebFrame())); IsFramePartOfMainTestWindow(frame->GetWebFrame()));
mock_screen_orientation_client_.OverrideAssociatedInterfaceProviderForFrame( mock_screen_orientation_client_.OverrideAssociatedInterfaceProviderForFrame(
frame->GetWebFrame()); frame->GetWebFrame());
......
...@@ -57,6 +57,7 @@ class BlinkTestRunner; ...@@ -57,6 +57,7 @@ class BlinkTestRunner;
class MockContentSettingsClient; class MockContentSettingsClient;
class MockScreenOrientationClient; class MockScreenOrientationClient;
class RenderFrame; class RenderFrame;
class SpellCheckClient;
class TestInterfaces; class TestInterfaces;
class WebFrameTestProxy; class WebFrameTestProxy;
class WebWidgetTestProxy; class WebWidgetTestProxy;
...@@ -78,7 +79,7 @@ class TestRunner { ...@@ -78,7 +79,7 @@ class TestRunner {
explicit TestRunner(TestInterfaces*); explicit TestRunner(TestInterfaces*);
virtual ~TestRunner(); virtual ~TestRunner();
void Install(WebFrameTestProxy* frame); void Install(WebFrameTestProxy* frame, SpellCheckClient* spell_check);
void SetDelegate(BlinkTestRunner*); void SetDelegate(BlinkTestRunner*);
void SetMainView(blink::WebView*); void SetMainView(blink::WebView*);
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "content/shell/renderer/web_test/blink_test_runner.h" #include "content/shell/renderer/web_test/blink_test_runner.h"
#include "content/shell/renderer/web_test/event_sender.h" #include "content/shell/renderer/web_test/event_sender.h"
#include "content/shell/renderer/web_test/gc_controller.h" #include "content/shell/renderer/web_test/gc_controller.h"
#include "content/shell/renderer/web_test/spell_check_client.h"
#include "content/shell/renderer/web_test/test_interfaces.h" #include "content/shell/renderer/web_test/test_interfaces.h"
#include "content/shell/renderer/web_test/test_plugin.h" #include "content/shell/renderer/web_test/test_plugin.h"
#include "content/shell/renderer/web_test/test_runner.h" #include "content/shell/renderer/web_test/test_runner.h"
...@@ -234,6 +235,9 @@ void WebFrameTestProxy::Initialize() { ...@@ -234,6 +235,9 @@ void WebFrameTestProxy::Initialize() {
GetWebFrame()->SetContentSettingsClient(test_runner->GetWebContentSettings()); GetWebFrame()->SetContentSettingsClient(test_runner->GetWebContentSettings());
spell_check_ = std::make_unique<SpellCheckClient>(GetWebFrame());
GetWebFrame()->SetTextCheckClient(spell_check_.get());
GetAssociatedInterfaceRegistry()->AddInterface( GetAssociatedInterfaceRegistry()->AddInterface(
base::BindRepeating(&WebFrameTestProxy::BindReceiver, base::BindRepeating(&WebFrameTestProxy::BindReceiver,
// The registry goes away and stops using this // The registry goes away and stops using this
...@@ -261,6 +265,8 @@ void WebFrameTestProxy::Reset() { ...@@ -261,6 +265,8 @@ void WebFrameTestProxy::Reset() {
if (IsLocalRoot()) { if (IsLocalRoot()) {
GetLocalRootWebWidgetTestProxy()->Reset(); GetLocalRootWebWidgetTestProxy()->Reset();
} }
spell_check_->Reset();
} }
std::string WebFrameTestProxy::GetFrameNameForWebTests() { std::string WebFrameTestProxy::GetFrameNameForWebTests() {
...@@ -698,7 +704,7 @@ void WebFrameTestProxy::DidClearWindowObject() { ...@@ -698,7 +704,7 @@ void WebFrameTestProxy::DidClearWindowObject() {
// frame before JS has a chance to run. // frame before JS has a chance to run.
GCController::Install(GetWebFrame()); GCController::Install(GetWebFrame());
interfaces->Install(GetWebFrame()); interfaces->Install(GetWebFrame());
test_runner()->Install(this); test_runner()->Install(this, spell_check_.get());
web_view_test_proxy_->Install(GetWebFrame()); web_view_test_proxy_->Install(GetWebFrame());
GetLocalRootWebWidgetTestProxy()->Install(GetWebFrame()); GetLocalRootWebWidgetTestProxy()->Install(GetWebFrame());
blink::WebTestingSupport::InjectInternalsObject(GetWebFrame()); blink::WebTestingSupport::InjectInternalsObject(GetWebFrame());
......
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
namespace content { namespace content {
class BlinkTestRunner; class BlinkTestRunner;
class SpellCheckClient;
class TestRunner; class TestRunner;
class WebViewTestProxy; class WebViewTestProxy;
class WebWidgetTestProxy; class WebWidgetTestProxy;
...@@ -106,6 +107,8 @@ class WebFrameTestProxy : public RenderFrameImpl, ...@@ -106,6 +107,8 @@ class WebFrameTestProxy : public RenderFrameImpl,
WebViewTestProxy* const web_view_test_proxy_; WebViewTestProxy* const web_view_test_proxy_;
std::unique_ptr<SpellCheckClient> spell_check_;
mojo::AssociatedReceiver<mojom::WebTestRenderFrame> mojo::AssociatedReceiver<mojom::WebTestRenderFrame>
web_test_render_frame_receiver_{this}; web_test_render_frame_receiver_{this};
......
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