Revert "Make input element UA shadow tree creation lazy"

This reverts commit 259286f. Reason for revert: Seems to be causing a bug with input placeholder text rendering: https://bugs.chromium.org/p/chromium/issues/detail?id=1421233#c7 Original change's description: > Make input element UA shadow tree creation lazy > > This CL comes from WebCore https://trac.webkit.org/changeset/290639/webkit, > it claims 0.5% progression on Speedometer2, the chromium pinpoint result > shows a 0.6% progression on Speedometer2. > > This CL lazy create shadow subtree until one of the following happens: > 1. the element is inserted into the document > 2. the type="" or value="" attributes are changed before the element is inserted into the document > 3. any DOM methods that need access to the InnerEditorElement() are called on the element before the element is inserted into the document > > > Change-Id: I5fd6743182d65be7e00fcb59480a5b28af289b76 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4111088 > Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1110526} Change-Id: Ic8042601bbdd1eedf36633a8fdc73d823d74fc4c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4329778 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Mason Freed <masonf@chromium.org> Cr-Commit-Position: refs/heads/main@{#1115897}
chromium · Mar 10, 2023 · dff1536 · dff1536
1 parent 316ac09
commit dff1536
Show file tree

Hide file tree

Showing 20 changed files with 44 additions and 106 deletions.
diff --git a/third_party/blink/renderer/core/html/forms/base_button_input_type.cc b/third_party/blink/renderer/core/html/forms/base_button_input_type.cc
@@ -65,7 +65,7 @@ BaseButtonInputType::SupportsPopoverTriggering() const {
 }
 
 void BaseButtonInputType::ValueAttributeChanged() {
-  To<Text>(GetElement().EnsureShadowSubtree()->firstChild())
+  To<Text>(GetElement().UserAgentShadowRoot()->firstChild())
       ->setData(GetElement().ValueOrDefaultLabel());
 }
 

diff --git a/third_party/blink/renderer/core/html/forms/chooser_only_temporal_input_type_view.cc b/third_party/blink/renderer/core/html/forms/chooser_only_temporal_input_type_view.cc
@@ -102,7 +102,7 @@ void ChooserOnlyTemporalInputTypeView::CreateShadowSubtree() {
 }
 
 void ChooserOnlyTemporalInputTypeView::UpdateView() {
-  Node* node = GetElement().EnsureShadowSubtree()->firstChild();
+  Node* node = GetElement().UserAgentShadowRoot()->firstChild();
   auto* html_element = DynamicTo<HTMLElement>(node);
   if (!html_element)
     return;

diff --git a/third_party/blink/renderer/core/html/forms/file_input_type.cc b/third_party/blink/renderer/core/html/forms/file_input_type.cc
@@ -363,14 +363,14 @@ void FileInputType::CreateShadowSubtree() {
 }
 
 HTMLInputElement* FileInputType::UploadButton() const {
-  Element* element = GetElement().EnsureShadowSubtree()->getElementById(
+  Element* element = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdFileUploadButton);
   CHECK(!element || IsA<HTMLInputElement>(element));
   return To<HTMLInputElement>(element);
 }
 
 Node* FileInputType::FileStatusElement() const {
-  return GetElement().EnsureShadowSubtree()->lastChild();
+  return GetElement().UserAgentShadowRoot()->lastChild();
 }
 
 void FileInputType::DisabledAttributeChanged() {

diff --git a/third_party/blink/renderer/core/html/forms/file_input_type_test.cc b/third_party/blink/renderer/core/html/forms/file_input_type_test.cc
@@ -243,7 +243,7 @@ TEST(FileInputTypeTest, ChangeTypeDuringOpeningFileChooser) {
   // Receiving a FileChooser response should not alter a shadow tree
   // for another type.
   EXPECT_TRUE(IsA<HTMLElement>(
-      input.EnsureShadowSubtree()->firstChild()->firstChild()));
+      input.UserAgentShadowRoot()->firstChild()->firstChild()));
 }
 
 // Tests selecting same file twice should fire cancel event second time.

diff --git a/third_party/blink/renderer/core/html/forms/html_input_element.cc b/third_party/blink/renderer/core/html/forms/html_input_element.cc
@@ -44,7 +44,6 @@
 #include "third_party/blink/renderer/core/css/style_change_reason.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/dom_token_list.h"
-#include "third_party/blink/renderer/core/dom/events/event_dispatch_forbidden_scope.h"
 #include "third_party/blink/renderer/core/dom/events/scoped_event_queue.h"
 #include "third_party/blink/renderer/core/dom/events/simulated_click_options.h"
 #include "third_party/blink/renderer/core/dom/id_target_observer.h"
@@ -149,6 +148,12 @@ HTMLInputElement::HTMLInputElement(Document& document,
                       : MakeGarbageCollected<TextInputType>(*this)),
       input_type_view_(input_type_ ? input_type_->CreateView() : nullptr) {
   SetHasCustomStyleCallbacks();
+
+  if (!flags.IsCreatedByParser()) {
+    DCHECK(input_type_view_->NeedsShadowSubtree());
+    CreateUserAgentShadowRoot();
+    CreateShadowSubtree();
+  }
 }
 
 void HTMLInputElement::Trace(Visitor* visitor) const {
@@ -378,7 +383,6 @@ void HTMLInputElement::HandleBlurEvent() {
 }
 
 void HTMLInputElement::setType(const AtomicString& type) {
-  EnsureShadowSubtree();
   setAttribute(html_names::kTypeAttr, type);
 }
 
@@ -396,6 +400,11 @@ void HTMLInputElement::InitializeTypeInParsing() {
     non_attribute_value_ = SanitizeValue(default_value);
   has_been_password_field_ |= new_type_name == input_type_names::kPassword;
 
+  if (input_type_view_->NeedsShadowSubtree()) {
+    CreateUserAgentShadowRoot();
+    CreateShadowSubtree();
+  }
+
   UpdateWillValidateCache();
 
   if (!default_value.IsNull())
@@ -468,8 +477,10 @@ void HTMLInputElement::UpdateType() {
   input_type_view_->WillBeDestroyed();
   input_type_ = new_type;
   input_type_view_ = input_type_->CreateView();
-
-  input_type_view_->CreateShadowSubtreeIfNeeded();
+  if (input_type_view_->NeedsShadowSubtree()) {
+    EnsureUserAgentShadowRoot();
+    CreateShadowSubtree();
+  }
 
   UpdateWillValidateCache();
 
@@ -1501,9 +1512,8 @@ void HTMLInputElement::DefaultEventHandler(Event& evt) {
     TextControlElement::DefaultEventHandler(evt);
 }
 
-ShadowRoot* HTMLInputElement::EnsureShadowSubtree() {
-  input_type_view_->CreateShadowSubtreeIfNeeded();
-  return UserAgentShadowRoot();
+void HTMLInputElement::CreateShadowSubtree() {
+  input_type_view_->CreateShadowSubtree();
 }
 
 bool HTMLInputElement::HasActivationBehavior() const {
@@ -1710,10 +1720,6 @@ Node::InsertionNotificationRequest HTMLInputElement::InsertedInto(
   ResetListAttributeTargetObserver();
   LogAddElementIfIsolatedWorldAndInDocument("input", html_names::kTypeAttr,
                                             html_names::kFormactionAttr);
-  {
-    EventDispatchForbiddenScope::AllowUserAgentEvents allow_events;
-    input_type_view_->CreateShadowSubtreeIfNeeded();
-  }
   return kInsertionShouldCallDidNotifySubtreeInsertions;
 }
 
@@ -1960,11 +1966,6 @@ bool HTMLInputElement::SupportsPlaceholder() const {
   return input_type_->SupportsPlaceholder();
 }
 
-TextControlInnerEditorElement* HTMLInputElement::EnsureInnerEditorElement()
-    const {
-  return input_type_view_->EnsureInnerEditorElement();
-}
-
 void HTMLInputElement::UpdatePlaceholderText() {
   return input_type_view_->UpdatePlaceholderText(!SuggestedValue().empty());
 }

diff --git a/third_party/blink/renderer/core/html/forms/html_input_element.h b/third_party/blink/renderer/core/html/forms/html_input_element.h
@@ -370,10 +370,9 @@ class CORE_EXPORT HTMLInputElement
   bool isMutable();
   void showPicker(ExceptionState&);
 
-  ShadowRoot* EnsureShadowSubtree();
-
  protected:
   void DefaultEventHandler(Event&) override;
+  void CreateShadowSubtree();
 
  private:
   enum AutoCompleteSetting { kUninitialized, kOn, kOff };
@@ -440,7 +439,6 @@ class CORE_EXPORT HTMLInputElement
   bool TooLong(const String&, NeedsToCheckDirtyFlag) const;
   bool TooShort(const String&, NeedsToCheckDirtyFlag) const;
 
-  TextControlInnerEditorElement* EnsureInnerEditorElement() const final;
   void UpdatePlaceholderText() final;
   bool IsEmptyValue() const final { return InnerEditorValue().empty(); }
   void HandleBlurEvent() final;

diff --git a/third_party/blink/renderer/core/html/forms/html_input_element_test.cc b/third_party/blink/renderer/core/html/forms/html_input_element_test.cc
@@ -150,7 +150,7 @@ TEST_F(HTMLInputElementTest, FilteredDataListOptionsDynamicContain) {
 TEST_F(HTMLInputElementTest, create) {
   auto* input = MakeGarbageCollected<HTMLInputElement>(
       GetDocument(), CreateElementFlags::ByCreateElement());
-  EXPECT_EQ(nullptr, input->UserAgentShadowRoot());
+  EXPECT_NE(nullptr, input->UserAgentShadowRoot());
 
   input = MakeGarbageCollected<HTMLInputElement>(
       GetDocument(), CreateElementFlags::ByParser(&GetDocument()));

diff --git a/third_party/blink/renderer/core/html/forms/html_text_area_element.cc b/third_party/blink/renderer/core/html/forms/html_text_area_element.cc
@@ -682,11 +682,6 @@ void HTMLTextAreaElement::SetPlaceholderVisibility(bool visible) {
   is_placeholder_visible_ = visible;
 }
 
-TextControlInnerEditorElement* HTMLTextAreaElement::EnsureInnerEditorElement()
-    const {
-  return InnerEditorElement();
-}
-
 void HTMLTextAreaElement::UpdatePlaceholderText() {
   HTMLElement* placeholder = PlaceholderElement();
   const String placeholder_text = GetPlaceholderValue();

diff --git a/third_party/blink/renderer/core/html/forms/html_text_area_element.h b/third_party/blink/renderer/core/html/forms/html_text_area_element.h
@@ -97,7 +97,6 @@ class CORE_EXPORT HTMLTextAreaElement final : public TextControlElement {
   String GetPlaceholderValue() const final;
   void UpdatePlaceholderText() override;
   bool IsEmptyValue() const override { return Value().empty(); }
-  TextControlInnerEditorElement* EnsureInnerEditorElement() const final;
 
   bool IsOptionalFormControl() const override {
     return !IsRequiredFormControl();

diff --git a/third_party/blink/renderer/core/html/forms/input_type_view.cc b/third_party/blink/renderer/core/html/forms/input_type_view.cc
@@ -130,22 +130,8 @@ bool InputTypeView::NeedsShadowSubtree() const {
   return true;
 }
 
-TextControlInnerEditorElement* InputTypeView::EnsureInnerEditorElement() {
-  CreateShadowSubtreeIfNeeded();
-  return GetElement().InnerEditorElement();
-}
-
 void InputTypeView::CreateShadowSubtree() {}
 
-void InputTypeView::CreateShadowSubtreeIfNeeded() {
-  if (has_created_shadow_subtree_ || !NeedsShadowSubtree()) {
-    return;
-  }
-  GetElement().EnsureUserAgentShadowRoot();
-  has_created_shadow_subtree_ = true;
-  CreateShadowSubtree();
-}
-
 void InputTypeView::DestroyShadowSubtree() {
   if (ShadowRoot* root = GetElement().UserAgentShadowRoot())
     root->RemoveChildren();

diff --git a/third_party/blink/renderer/core/html/forms/input_type_view.h b/third_party/blink/renderer/core/html/forms/input_type_view.h
@@ -59,7 +59,6 @@ class KeyboardEvent;
 class LayoutObject;
 enum class LegacyLayout;
 class MouseEvent;
-class TextControlInnerEditorElement;
 
 class ClickHandlingState final : public EventDispatchHandlingState {
  public:
@@ -120,9 +119,6 @@ class CORE_EXPORT InputTypeView : public GarbageCollectedMixin {
 
   // Functions for shadow trees
 
-  TextControlInnerEditorElement* EnsureInnerEditorElement();
-  bool HasCreatedShadowSubtree() const { return has_created_shadow_subtree_; }
-  void CreateShadowSubtreeIfNeeded();
   virtual bool NeedsShadowSubtree() const;
   virtual void CreateShadowSubtree();
   virtual void DestroyShadowSubtree();
@@ -165,7 +161,6 @@ class CORE_EXPORT InputTypeView : public GarbageCollectedMixin {
   bool will_be_destroyed_ = false;
 
  private:
-  bool has_created_shadow_subtree_ = false;
   Member<HTMLInputElement> element_;
 };
 

diff --git a/third_party/blink/renderer/core/html/forms/multiple_fields_temporal_input_type_view.cc b/third_party/blink/renderer/core/html/forms/multiple_fields_temporal_input_type_view.cc
@@ -142,39 +142,39 @@ bool DateTimeFormatValidator::ValidateFormat(
 
 DateTimeEditElement*
 MultipleFieldsTemporalInputTypeView::GetDateTimeEditElement() const {
-  auto* element = GetElement().EnsureShadowSubtree()->getElementById(
+  auto* element = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdDateTimeEdit);
   CHECK(!element || IsA<DateTimeEditElement>(element));
   return To<DateTimeEditElement>(element);
 }
 
 SpinButtonElement* MultipleFieldsTemporalInputTypeView::GetSpinButtonElement()
     const {
-  auto* element = GetElement().EnsureShadowSubtree()->getElementById(
+  auto* element = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdSpinButton);
   CHECK(!element || IsA<SpinButtonElement>(element));
   return To<SpinButtonElement>(element);
 }
 
 ClearButtonElement* MultipleFieldsTemporalInputTypeView::GetClearButtonElement()
     const {
-  auto* element = GetElement().EnsureShadowSubtree()->getElementById(
+  auto* element = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdClearButton);
   CHECK(!element || IsA<ClearButtonElement>(element));
   return To<ClearButtonElement>(element);
 }
 
 PickerIndicatorElement*
 MultipleFieldsTemporalInputTypeView::GetPickerIndicatorElement() const {
-  auto* element = GetElement().EnsureShadowSubtree()->getElementById(
+  auto* element = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdPickerIndicator);
   CHECK(!element || IsA<PickerIndicatorElement>(element));
   return To<PickerIndicatorElement>(element);
 }
 
 inline bool MultipleFieldsTemporalInputTypeView::ContainsFocusedShadowElement()
     const {
-  return GetElement().EnsureShadowSubtree()->contains(
+  return GetElement().UserAgentShadowRoot()->contains(
       GetElement().GetDocument().FocusedElement());
 }
 

diff --git a/third_party/blink/renderer/core/html/forms/password_input_type.cc b/third_party/blink/renderer/core/html/forms/password_input_type.cc
@@ -152,7 +152,7 @@ bool PasswordInputType::ShouldDrawCapsLockIndicator() const {
 }
 
 void PasswordInputType::UpdatePasswordRevealButton() {
-  Element* button = GetElement().EnsureShadowSubtree()->getElementById(
+  Element* button = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdPasswordRevealButton);
 
   // Update the glyph.

diff --git a/third_party/blink/renderer/core/html/forms/range_input_type.cc b/third_party/blink/renderer/core/html/forms/range_input_type.cc
@@ -152,10 +152,6 @@ StepRange RangeInputType::CreateStepRange(
 }
 
 void RangeInputType::HandleMouseDownEvent(MouseEvent& event) {
-  if (!HasCreatedShadowSubtree()) {
-    return;
-  }
-
   if (GetElement().IsDisabledFormControl())
     return;
 
@@ -303,9 +299,7 @@ ControlPart RangeInputType::AutoAppearance() const {
 }
 
 void RangeInputType::UpdateView() {
-  if (HasCreatedShadowSubtree()) {
-    GetSliderThumbElement()->SetPositionFromValue();
-  }
+  GetSliderThumbElement()->SetPositionFromValue();
 }
 
 String RangeInputType::SanitizeValue(const String& proposed_value) const {
@@ -323,9 +317,6 @@ void RangeInputType::WarnIfValueIsInvalid(const String& value) const {
 }
 
 void RangeInputType::DisabledAttributeChanged() {
-  if (!HasCreatedShadowSubtree()) {
-    return;
-  }
   if (GetElement().IsDisabledFormControl())
     GetSliderThumbElement()->StopDragging();
 }
@@ -341,10 +332,6 @@ inline SliderThumbElement* RangeInputType::GetSliderThumbElement() const {
 }
 
 inline Element* RangeInputType::SliderTrackElement() const {
-  if (!HasCreatedShadowSubtree()) {
-    return nullptr;
-  }
-
   return GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdSliderTrack);
 }
@@ -354,7 +341,7 @@ void RangeInputType::ListAttributeTargetChanged() {
   if (auto* object = GetElement().GetLayoutObject())
     object->SetSubtreeShouldDoFullPaintInvalidation();
   Element* slider_track_element = SliderTrackElement();
-  if (slider_track_element && slider_track_element->GetLayoutObject()) {
+  if (slider_track_element->GetLayoutObject()) {
     slider_track_element->GetLayoutObject()->SetNeedsLayout(
         layout_invalidation_reason::kAttributeChanged);
   }

diff --git a/third_party/blink/renderer/core/html/forms/search_input_type.cc b/third_party/blink/renderer/core/html/forms/search_input_type.cc
@@ -147,7 +147,7 @@ void SearchInputType::UpdateView() {
 }
 
 void SearchInputType::UpdateCancelButtonVisibility() {
-  Element* button = GetElement().EnsureShadowSubtree()->getElementById(
+  Element* button = GetElement().UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdSearchClearButton);
   if (!button)
     return;

diff --git a/third_party/blink/renderer/core/html/forms/slider_thumb_element.cc b/third_party/blink/renderer/core/html/forms/slider_thumb_element.cc
@@ -95,7 +95,7 @@ void SliderThumbElement::DragFrom(const LayoutPoint& point) {
 
 void SliderThumbElement::SetPositionFromPoint(const LayoutPoint& point) {
   HTMLInputElement* input(HostInput());
-  Element* track_element = input->EnsureShadowSubtree()->getElementById(
+  Element* track_element = input->UserAgentShadowRoot()->getElementById(
       shadow_element_names::kIdSliderTrack);
 
   const LayoutObject* input_object = input->GetLayoutObject();
@@ -329,10 +329,8 @@ void SliderContainerElement::DefaultEventHandler(Event& event) {
 
 void SliderContainerElement::HandleTouchEvent(TouchEvent* event) {
   HTMLInputElement* input = HostInput();
-  if (!input || !input->UserAgentShadowRoot() ||
-      input->IsDisabledFormControl() || !event) {
+  if (!input || input->IsDisabledFormControl() || !event)
     return;
-  }
 
   if (event->type() == event_type_names::kTouchend) {
     // TODO: Also do this for touchcancel?