Commit 8aeed115 authored by Kent Tamura's avatar Kent Tamura Committed by Commit Bot

input[type=file] should not alter the user-agent shadow tree for another type

We do something like:
  shadow_root.lastChild.textContent = 'foo.txt';
when input[type=file] receives the result from a file chooser dialog.

If a file input requests to open a file chooser and the input type is
changed before receiving the result, we remove shadow elements by the
textContent setter in some cases.

This CL fixes it by
 - Introduce InputTypeView::will_be_destroyed_, which becomes true when
   a InputTypeView is disconnected from the owner HTMLInputElement.
 - Check will_be_destroyed_ in functions which receive asynchronous
   results from pickers.

Bug: 1116378
Change-Id: I76fb166a0e5221441fcabeea34ab8a5bbd7d4af1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358718
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Auto-Submit: Kent Tamura <tkent@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798962}
parent 1e069b0a
......@@ -126,11 +126,15 @@ Element& ChooserOnlyTemporalInputTypeView::OwnerElement() const {
}
void ChooserOnlyTemporalInputTypeView::DidChooseValue(const String& value) {
if (will_be_destroyed_)
return;
GetElement().setValue(value,
TextFieldEventBehavior::kDispatchInputAndChangeEvent);
}
void ChooserOnlyTemporalInputTypeView::DidChooseValue(double value) {
if (will_be_destroyed_)
return;
DCHECK(std::isfinite(value) || std::isnan(value));
if (std::isnan(value)) {
GetElement().setValue(g_empty_string,
......
......@@ -202,7 +202,8 @@ void ColorInputType::ValueAttributeChanged() {
}
void ColorInputType::DidChooseColor(const Color& color) {
if (GetElement().IsDisabledFormControl() || color == ValueAsColor())
if (will_be_destroyed_ || GetElement().IsDisabledFormControl() ||
color == ValueAsColor())
return;
EventQueueScope scope;
GetElement().SetValueFromRenderer(color.Serialized());
......
......@@ -426,7 +426,8 @@ void FileInputType::FilesChosen(FileChooserFileInfoList files,
}
++i;
}
SetFilesAndDispatchEvents(CreateFileList(files, base_dir));
if (!will_be_destroyed_)
SetFilesAndDispatchEvents(CreateFileList(files, base_dir));
if (HasConnectedFileChooser())
DisconnectFileChooser();
}
......
......@@ -9,10 +9,13 @@
#include "third_party/blink/renderer/core/clipboard/data_object.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/fileapi/file_list.h"
#include "third_party/blink/renderer/core/frame/frame_test_helpers.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
#include "third_party/blink/renderer/core/html/forms/html_input_element.h"
#include "third_party/blink/renderer/core/html/forms/mock_file_chooser.h"
#include "third_party/blink/renderer/core/html_names.h"
#include "third_party/blink/renderer/core/input_type_names.h"
#include "third_party/blink/renderer/core/loader/empty_clients.h"
#include "third_party/blink/renderer/core/page/drag_data.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
......@@ -197,4 +200,37 @@ TEST(FileInputTypeTest, BeforePseudoCrash) {
// The test passes if no CHECK failures and no null pointer dereferences.
}
TEST(FileInputTypeTest, ChangeTypeDuringOpeningFileChooser) {
// We use WebViewHelper instead of DummyPageHolder, in order to use
// ChromeClientImpl.
frame_test_helpers::WebViewHelper helper;
helper.Initialize();
LocalFrame* frame = helper.LocalMainFrame()->GetFrame();
Document& doc = *frame->GetDocument();
doc.body()->setInnerHTML("<input type=file>");
auto& input = *To<HTMLInputElement>(doc.body()->firstChild());
base::RunLoop run_loop;
MockFileChooser chooser(frame->GetBrowserInterfaceBroker(),
run_loop.QuitClosure());
// Calls MockFileChooser::OpenFileChooser().
LocalFrame::NotifyUserActivation(
frame, mojom::blink::UserActivationNotificationType::kInteraction);
input.click();
run_loop.Run();
input.setType(input_type_names::kColor);
FileChooserFileInfoList list;
list.push_back(CreateFileChooserFileInfoNative("/path/to/file.txt", ""));
chooser.ResponseOnOpenFileChooser(std::move(list));
// Receiving a FileChooser response should not alter a shadow tree
// for another type.
EXPECT_TRUE(IsA<HTMLElement>(
input.UserAgentShadowRoot()->firstChild()->firstChild()));
}
} // namespace blink
......@@ -457,6 +457,7 @@ void HTMLInputElement::UpdateType() {
// to the element, and false otherwise.
const bool previously_selectable = input_type_->SupportsSelectionAPI();
input_type_view_->WillBeDestroyed();
input_type_ = new_type;
input_type_view_ = input_type_->CreateView();
if (input_type_view_->NeedsShadowSubtree()) {
......
......@@ -39,6 +39,10 @@
namespace blink {
void InputTypeView::WillBeDestroyed() {
will_be_destroyed_ = true;
}
InputTypeView::~InputTypeView() = default;
void InputTypeView::Trace(Visitor* visitor) const {
......
......@@ -71,6 +71,9 @@ class ClickHandlingState final : public EventDispatchHandlingState {
// derived from it to classes other than HTMLInputElement.
class CORE_EXPORT InputTypeView : public GarbageCollectedMixin {
public:
// Called by the owner HTMLInputElement when this InputType is disconnected
// from the HTMLInputElement.
void WillBeDestroyed();
virtual ~InputTypeView();
void Trace(Visitor*) const override;
......@@ -148,6 +151,8 @@ class CORE_EXPORT InputTypeView : public GarbageCollectedMixin {
InputTypeView(HTMLInputElement& element) : element_(&element) {}
HTMLInputElement& GetElement() const { return *element_; }
bool will_be_destroyed_ = false;
private:
Member<HTMLInputElement> element_;
......
......@@ -280,6 +280,8 @@ bool MultipleFieldsTemporalInputTypeView::
void MultipleFieldsTemporalInputTypeView::PickerIndicatorChooseValue(
const String& value) {
if (will_be_destroyed_)
return;
if (GetElement().IsValidValue(value)) {
GetElement().setValue(value, TextFieldEventBehavior::kDispatchInputEvent);
return;
......@@ -307,6 +309,8 @@ void MultipleFieldsTemporalInputTypeView::PickerIndicatorChooseValue(
void MultipleFieldsTemporalInputTypeView::PickerIndicatorChooseValue(
double value) {
if (will_be_destroyed_)
return;
DCHECK(std::isfinite(value) || std::isnan(value));
if (std::isnan(value)) {
GetElement().setValue(g_empty_string,
......
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