Commit 7d16fad1 authored by Keishi Hattori's avatar Keishi Hattori Committed by Commit Bot

Remove prefinalizer from ColorChooserPopupUIController

ColorChooserPopupUIController's prefinalizer closing the popup allocates new objects which is not allowed.

ColorChooserPopupUIController's prefinalizer which closes the popup should not exist.
In normal use, the popup will be explicitly closed before ColorChooserPopupUIController is destroyed.
The only exception is the Internals::endColorChooser() which releases the ColorChooser without closing the popup.

This CL fixes Internals::endColorChooser().

Bug: 1040389, 1039092
Change-Id: Ib52c80e5288ac7a115409706c96c3ba226357dd5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2004365
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732736}
parent db29a5ee
......@@ -54,12 +54,8 @@ ColorChooserPopupUIController::ColorChooserPopupUIController(
popup_(nullptr),
locale_(Locale::DefaultLocale()) {}
ColorChooserPopupUIController::~ColorChooserPopupUIController() = default;
void ColorChooserPopupUIController::Dispose() {
// Finalized earlier so as to access chrome_client_ while alive.
CancelPopup();
// ~ColorChooserUIController calls EndChooser().
ColorChooserPopupUIController::~ColorChooserPopupUIController() {
DCHECK(!popup_);
}
void ColorChooserPopupUIController::Trace(Visitor* visitor) {
......
......@@ -39,8 +39,6 @@ class PagePopup;
class CORE_EXPORT ColorChooserPopupUIController final
: public ColorChooserUIController,
public PagePopupClient {
USING_PRE_FINALIZER(ColorChooserPopupUIController, Dispose);
public:
ColorChooserPopupUIController(LocalFrame*,
ChromeClient*,
......@@ -69,7 +67,6 @@ class CORE_EXPORT ColorChooserPopupUIController final
ChromeClient& GetChromeClient() override;
void OpenPopup();
void Dispose();
void WriteColorPickerDocument(SharedBuffer*);
void WriteColorSuggestionPickerDocument(SharedBuffer*);
......
......@@ -1654,9 +1654,8 @@ void HTMLInputElement::SelectColorInColorChooser(const Color& color) {
client->DidChooseColor(color);
}
void HTMLInputElement::EndColorChooser() {
if (ColorChooserClient* client = input_type_->GetColorChooserClient())
client->DidEndChooser();
void HTMLInputElement::EndColorChooserForTesting() {
input_type_view_->ClosePopupView();
}
HTMLElement* HTMLInputElement::list() const {
......
......@@ -261,7 +261,7 @@ class CORE_EXPORT HTMLInputElement
// For test purposes.
void SelectColorInColorChooser(const Color&);
void EndColorChooser();
void EndColorChooserForTesting();
String DefaultToolTip() const override;
......
......@@ -852,7 +852,7 @@ void Internals::selectColorInColorChooser(Element* element,
void Internals::endColorChooser(Element* element) {
DCHECK(element);
if (auto* input = DynamicTo<HTMLInputElement>(*element))
input->EndColorChooser();
input->EndColorChooserForTesting();
}
bool Internals::hasAutofocusRequest(Document* document) {
......
......@@ -27,9 +27,13 @@ input.oninput = function() {
debug("input event dispatched - value is: " + input.value);
};
eventSender.mouseMoveTo(10, 10);
eventSender.mouseDown();
eventSender.mouseUp();
function openColorChooser() {
eventSender.mouseMoveTo(10, 10);
eventSender.mouseDown();
eventSender.mouseUp();
}
openColorChooser();
// Test that incorrect element arguments are (not) handled.
shouldThrow("internals.selectColorInColorChooser({}, '#ff0000');");
......@@ -45,6 +49,7 @@ function step1() {
function step2() {
debug("change event dispatched - value changed to " + input.value);
openColorChooser();
// input.onchange should not be called
input.onchange = () => {
testFailed("Change event should not be dispatched.");
......@@ -57,6 +62,7 @@ function step2() {
function step3() {
debug('Change event is only dispatched, when color chooser is closed');
openColorChooser();
input.onchange = step4;
internals.selectColorInColorChooser(input, '#ff0002');
internals.endColorChooser(input);
......
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