Commit e8c722f7 authored by Samuel Huang's avatar Samuel Huang Committed by Commit Bot

Revert "Use a Persistent FileChooserClient to avoid GC"

This reverts commit fee1e3c8.

Reason for revert: Suspected of causing memory leak in various tests (http://crbug.com/972005) e.g., crbgug.
* fast/forms/file/file-input-click.html
* fast/forms/file/file-input-key-enter.html
...


Original change's description:
> Use a Persistent FileChooserClient to avoid GC
> 
> Previous to this change, the FileChooserClient was only held by
> a WeakPersistent pointer, and as a result, the client could get
> garbage collected. This led to user-visible bugs, such as
> crbug.com/844119. This is now corrected.
> 
> Bug: 844119
> Change-Id: I7ab788e85dba4d6343a3abceafb092ef1cfe64f9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1639046
> Commit-Queue: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Auto-Submit: Mason Freed <masonfreed@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#666970}

TBR=tkent@chromium.org,masonfreed@chromium.org

Change-Id: Ief378adea1a4e19cf3fe0fe0a4b2517edeb8529f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 844119
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1650020Reviewed-by: default avatarSamuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667124}
parent af973ee8
...@@ -1868,7 +1868,6 @@ jumbo_source_set("unit_tests") { ...@@ -1868,7 +1868,6 @@ jumbo_source_set("unit_tests") {
"html/custom/custom_element_upgrade_sorter_test.cc", "html/custom/custom_element_upgrade_sorter_test.cc",
"html/forms/email_input_type_test.cc", "html/forms/email_input_type_test.cc",
"html/forms/external_popup_menu_test.cc", "html/forms/external_popup_menu_test.cc",
"html/forms/file_chooser_test.cc",
"html/forms/file_input_type_test.cc", "html/forms/file_input_type_test.cc",
"html/forms/form_controller_test.cc", "html/forms/form_controller_test.cc",
"html/forms/form_data_test.cc", "html/forms/form_data_test.cc",
......
...@@ -96,7 +96,7 @@ class FileChooser : public RefCounted<FileChooser> { ...@@ -96,7 +96,7 @@ class FileChooser : public RefCounted<FileChooser> {
// mojom::blink::FileChooser callback // mojom::blink::FileChooser callback
void DidChooseFiles(mojom::blink::FileChooserResultPtr result); void DidChooseFiles(mojom::blink::FileChooserResultPtr result);
Persistent<FileChooserClient> client_; WeakPersistent<FileChooserClient> client_;
mojom::blink::FileChooserParamsPtr params_; mojom::blink::FileChooserParamsPtr params_;
Persistent<ChromeClientImpl> chrome_client_impl_; Persistent<ChromeClientImpl> chrome_client_impl_;
mojom::blink::FileChooserPtr file_chooser_; mojom::blink::FileChooserPtr file_chooser_;
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/core/html/forms/file_chooser.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/core/html/forms/mock_file_chooser.h"
#include "third_party/blink/renderer/core/page/chrome_client_impl.h"
namespace blink {
class FileChooserTest : public testing::Test {
protected:
void SetUp() override { web_view_ = helper_.Initialize(); }
frame_test_helpers::WebViewHelper helper_;
WebViewImpl* web_view_;
};
TEST_F(FileChooserTest, NotGarbageCollected) {
LocalFrame* frame = helper_.LocalMainFrame()->GetFrame();
auto* client = MakeGarbageCollected<MockFileChooserClient>(frame);
mojom::blink::FileChooserParams params;
params.title = g_empty_string;
scoped_refptr<FileChooser> chooser = FileChooser::Create(client, params);
ThreadState::Current()->CollectAllGarbageForTesting();
// This tests the nullness of FileChooser::client_.
EXPECT_TRUE(chooser->FrameOrNull());
}
} // namespace blink
...@@ -74,28 +74,5 @@ class MockFileChooser : public mojom::blink::FileChooser { ...@@ -74,28 +74,5 @@ class MockFileChooser : public mojom::blink::FileChooser {
base::OnceClosure reached_callback_; base::OnceClosure reached_callback_;
}; };
// A FileChooserClient which makes FileChooser::OpenFileChooser() success.
class MockFileChooserClient
: public GarbageCollectedFinalized<MockFileChooserClient>,
public FileChooserClient {
USING_GARBAGE_COLLECTED_MIXIN(MockFileChooserClient);
public:
explicit MockFileChooserClient(LocalFrame* frame) : frame_(frame) {}
void Trace(Visitor* visitor) override {
visitor->Trace(frame_);
FileChooserClient::Trace(visitor);
}
private:
// FilesChosen() and WillOpenPopup() are never called in the test.
void FilesChosen(FileChooserFileInfoList, const base::FilePath&) override {}
void WillOpenPopup() override {}
LocalFrame* FrameOrNull() const override { return frame_; }
Member<LocalFrame> frame_;
};
} // namespace blink } // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_CORE_HTML_FORMS_MOCK_FILE_CHOOSER_H_ #endif // THIRD_PARTY_BLINK_RENDERER_CORE_HTML_FORMS_MOCK_FILE_CHOOSER_H_
...@@ -230,6 +230,29 @@ TEST_F(PagePopupSuppressionTest, SuppressDateTimeChooser) { ...@@ -230,6 +230,29 @@ TEST_F(PagePopupSuppressionTest, SuppressDateTimeChooser) {
EXPECT_TRUE(CanOpenDateTimeChooser()); EXPECT_TRUE(CanOpenDateTimeChooser());
} }
// A FileChooserClient which makes FileChooser::OpenFileChooser() success.
class MockFileChooserClient
: public GarbageCollectedFinalized<MockFileChooserClient>,
public FileChooserClient {
USING_GARBAGE_COLLECTED_MIXIN(MockFileChooserClient);
public:
explicit MockFileChooserClient(LocalFrame* frame) : frame_(frame) {}
void Trace(Visitor* visitor) override {
visitor->Trace(frame_);
FileChooserClient::Trace(visitor);
}
private:
// FilesChosen() and WillOpenPopup() are never called in the test.
void FilesChosen(FileChooserFileInfoList, const base::FilePath&) override {}
void WillOpenPopup() override {}
LocalFrame* FrameOrNull() const override { return frame_; }
Member<LocalFrame> frame_;
};
class FileChooserQueueTest : public testing::Test { class FileChooserQueueTest : public testing::Test {
protected: protected:
void SetUp() override { void SetUp() override {
......
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