Bug 1599971 - part 2: Mark `TextControlState::DeleteOrCacheForReuse()` and `TextControlState::Reuse()` as `MOZ_CAN_RUN_SCRIPT` r=m_kato! draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 04 Dec 2019 12:32:00 +0900
changeset 2514399 a5472ee9c917b8d7a2460e6aec8c0618cf6c7d87
parent 2514398 adc33be8f955dfd479862ded6f7e44be6ec2f060
child 2514400 974adaa889bdcb3b266cb6a38a5d4cefbaeecbac
push id459907
push usermasayuki@d-toybox.com
push dateWed, 04 Dec 2019 12:49:29 +0000
treeherdertry@ee01a7f649cb [default view] [failures only]
reviewersm_kato
bugs1599971
milestone73.0a1
Bug 1599971 - part 2: Mark `TextControlState::DeleteOrCacheForReuse()` and `TextControlState::Reuse()` as `MOZ_CAN_RUN_SCRIPT` r=m_kato! They are really complicated and can run script while the instance is being destroyed. Therefore, they should have their own `AutoTextControlHandlingState` as "kungFuDeathGrip". Differential Revision: https://phabricator.services.mozilla.com/D55773
dom/html/HTMLInputElement.h
dom/html/HTMLTextAreaElement.h
dom/html/TextControlState.cpp
dom/html/TextControlState.h
--- a/dom/html/HTMLInputElement.h
+++ b/dom/html/HTMLInputElement.h
@@ -870,17 +870,17 @@ class HTMLInputElement final : public Te
    * that @required attribute applies and the attribute is set; in contrast,
    * Required() returns true whenever @required attribute is set.
    */
   bool IsRequired() const { return State().HasState(NS_EVENT_STATE_REQUIRED); }
 
   bool HasBeenTypePassword() { return mHasBeenTypePassword; }
 
  protected:
-  virtual ~HTMLInputElement();
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual ~HTMLInputElement();
 
   virtual JSObject* WrapNode(JSContext* aCx,
                              JS::Handle<JSObject*> aGivenProto) override;
 
   // Pull IsSingleLineTextControl into our scope, otherwise it'd be hidden
   // by the TextControlElement version.
   using nsGenericHTMLFormElementWithState::IsSingleLineTextControl;
 
@@ -1090,17 +1090,17 @@ class HTMLInputElement final : public Te
    */
   bool DoesValueAsNumberApply() const { return DoesMinMaxApply(); }
 
   /**
    * Returns if autocomplete attribute applies for the current type.
    */
   bool DoesAutocompleteApply() const;
 
-  void FreeData();
+  MOZ_CAN_RUN_SCRIPT void FreeData();
   TextControlState* GetEditorState() const;
 
   mozilla::TextEditor* GetTextEditorFromState();
 
   /**
    * Manages the internal data storage across type changes.
    */
   MOZ_CAN_RUN_SCRIPT
--- a/dom/html/HTMLTextAreaElement.h
+++ b/dom/html/HTMLTextAreaElement.h
@@ -278,17 +278,17 @@ class HTMLTextAreaElement final : public
   }
 
   bool IsInputEventTarget() const { return true; }
 
   MOZ_CAN_RUN_SCRIPT_BOUNDARY void SetUserInput(
       const nsAString& aValue, nsIPrincipal& aSubjectPrincipal);
 
  protected:
-  virtual ~HTMLTextAreaElement();
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY virtual ~HTMLTextAreaElement();
 
   // get rid of the compiler warning
   using nsGenericHTMLFormElementWithState::IsSingleLineTextControl;
 
   virtual JSObject* WrapNode(JSContext* aCx,
                              JS::Handle<JSObject*> aGivenProto) override;
 
   nsCOMPtr<nsIControllers> mControllers;
--- a/dom/html/TextControlState.cpp
+++ b/dom/html/TextControlState.cpp
@@ -1070,21 +1070,24 @@ nsresult TextInputListener::UpdateTextIn
  *
  * This class is temporarily created in the stack and can manage nested
  * handling state of TextControlState.  While this instance exists, lifetime of
  * TextControlState which created the instance is guaranteed.  In other words,
  * you can use this class as "kungFuDeathGrip" for TextControlState.
  *****************************************************************************/
 
 enum class TextControlAction {
+  CacheForReuse,
   CommitComposition,
+  Destructor,
   PrepareEditor,
   SetRangeText,
   SetSelectionRange,
   SetValue,
+  Unlink,
 };
 
 class MOZ_STACK_CLASS AutoTextControlHandlingState {
  public:
   AutoTextControlHandlingState() = delete;
   explicit AutoTextControlHandlingState(const AutoTextControlHandlingState&) =
       delete;
   AutoTextControlHandlingState(AutoTextControlHandlingState&&) = delete;
@@ -1155,16 +1158,22 @@ class MOZ_STACK_CLASS AutoTextControlHan
     }
     if (!mTextControlStateDestroyed && mPreareEditorLater) {
       MOZ_ASSERT(nsContentUtils::IsSafeToRunScript());
       mTextControlState.PrepareEditor();
     }
   }
 
   void OnDestroyTextControlState() {
+    if (IsHandling(TextControlAction::Destructor) ||
+        IsHandling(TextControlAction::CacheForReuse)) {
+      // Do nothing since mTextContrlState.DeleteOrCacheForReuse() has
+      // already been called.
+      return;
+    }
     mTextControlStateDestroyed = true;
     if (mParent) {
       mParent->OnDestroyTextControlState();
     }
   }
 
   void PrepareEditorLater() {
     MOZ_ASSERT(IsHandling(TextControlAction::SetValue));
@@ -1387,16 +1396,18 @@ TextControlState* TextControlState::Cons
   }
 
   return new TextControlState(aOwningElement);
 }
 
 TextControlState::~TextControlState() {
   MOZ_ASSERT(!mHandlingState);
   MOZ_COUNT_DTOR(TextControlState);
+  AutoTextControlHandlingState handlingDesctructor(
+      *this, TextControlAction::Destructor);
   Clear();
 }
 
 void TextControlState::Shutdown() {
   sHasShutDown = true;
   if (sReleasedInstances) {
     for (TextControlState* textControlState : *sReleasedInstances) {
       textControlState->DeleteOrCacheForReuse();
@@ -1407,30 +1418,43 @@ void TextControlState::Shutdown() {
 
 void TextControlState::Destroy() {
   // If we're handling something, we should be deleted later.
   if (mHandlingState) {
     mHandlingState->OnDestroyTextControlState();
     return;
   }
   DeleteOrCacheForReuse();
+  // Note that this instance may have already been deleted here.  Don't touch
+  // any members.
 }
 
 void TextControlState::DeleteOrCacheForReuse() {
   MOZ_ASSERT(!IsBusy());
 
   // If we can cache this instance, we should do it instead of deleting it.
   if (!sHasShutDown && (!sReleasedInstances || sReleasedInstances->Length() <
                                                    kMaxCountOfCacheToReuse)) {
-    PrepareForReuse();
+    AutoTextControlHandlingState handlingCacheForReuse(
+        *this, TextControlAction::CacheForReuse);
+
+    // Prepare for reuse, unlink and release any refcountable objects.
+    UnlinkInternal();
+    mValue.reset();
+    mTextCtrlElement = nullptr;
+
+    // Put this instance to the cache.  Note that now, the array may be full,
+    // but it's not problem to cache more instances than kMaxCountOfCacheToReuse
+    // because it just requires reallocation cost of the array buffer.
     if (!sReleasedInstances) {
       sReleasedInstances =
           new AutoTArray<TextControlState*, kMaxCountOfCacheToReuse>;
     }
     sReleasedInstances->AppendElement(this);
+
     return;
   }
   delete this;
 }
 
 nsresult TextControlState::OnEditActionHandled() {
   return mHandlingState ? mHandlingState->OnEditActionHandled() : NS_OK;
 }
@@ -1439,25 +1463,24 @@ Element* TextControlState::GetRootNode()
   return mBoundFrame ? mBoundFrame->GetRootNode() : nullptr;
 }
 
 Element* TextControlState::GetPreviewNode() {
   return mBoundFrame ? mBoundFrame->GetPreviewNode() : nullptr;
 }
 
 void TextControlState::Clear() {
+  MOZ_ASSERT(mHandlingState);
+  MOZ_ASSERT(mHandlingState->Is(TextControlAction::Destructor) ||
+             mHandlingState->Is(TextControlAction::CacheForReuse) ||
+             mHandlingState->Is(TextControlAction::Unlink));
   if (mTextEditor) {
     mTextEditor->SetTextInputListener(nullptr);
   }
 
-  if (mHandlingState) {
-    mHandlingState->OnDestroyTextControlState();
-    mHandlingState = nullptr;
-  }
-
   if (mBoundFrame) {
     // Oops, we still have a frame!
     // This should happen when the type of a text input control is being changed
     // to something which is not a text control.  In this case, we should
     // pretend that a frame is being destroyed, and clean up after ourselves
     // properly.
     UnbindFromFrame(mBoundFrame);
     mTextEditor = nullptr;
@@ -1465,16 +1488,24 @@ void TextControlState::Clear() {
     // If we have a bound frame around, UnbindFromFrame will call DestroyEditor
     // for us.
     DestroyEditor();
   }
   mTextListener = nullptr;
 }
 
 void TextControlState::Unlink() {
+  AutoTextControlHandlingState handlingUnlink(*this, TextControlAction::Unlink);
+  UnlinkInternal();
+}
+
+void TextControlState::UnlinkInternal() {
+  MOZ_ASSERT(mHandlingState);
+  MOZ_ASSERT(mHandlingState->Is(TextControlAction::Unlink) ||
+             mHandlingState->Is(TextControlAction::CacheForReuse));
   TextControlState* tmp = this;
   tmp->Clear();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mSelCon)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mTextEditor)
 }
 
 void TextControlState::Traverse(nsCycleCollectionTraversalCallback& cb) {
   TextControlState* tmp = this;
--- a/dom/html/TextControlState.h
+++ b/dom/html/TextControlState.h
@@ -143,17 +143,17 @@ class TextControlState final : public Su
 
   // Note that this does not run script actually because of `sHasShutDown`
   // is set to true before calling `DeleteOrCacheForReuse()`.
   MOZ_CAN_RUN_SCRIPT_BOUNDARY static void Shutdown();
 
   /**
    * Destroy() deletes the instance immediately or later.
    */
-  void Destroy();
+  MOZ_CAN_RUN_SCRIPT void Destroy();
 
   TextControlState() = delete;
   explicit TextControlState(const TextControlState&) = delete;
   TextControlState(TextControlState&&) = delete;
 
   void operator=(const TextControlState&) = delete;
   void operator=(TextControlState&&) = delete;
 
@@ -401,29 +401,24 @@ class TextControlState final : public Su
   void UpdateEditableState(bool aNotify) {
     if (auto* root = GetRootNode()) {
       root->UpdateEditableState(aNotify);
     }
   }
 
  private:
   explicit TextControlState(TextControlElement* aOwningElement);
-  MOZ_CAN_RUN_SCRIPT_BOUNDARY ~TextControlState();
+  MOZ_CAN_RUN_SCRIPT ~TextControlState();
 
   /**
    * Delete the instance or cache to reuse it if possible.
    */
-  void DeleteOrCacheForReuse();
+  MOZ_CAN_RUN_SCRIPT void DeleteOrCacheForReuse();
 
-  void PrepareForReuse() {
-    MOZ_ASSERT(!IsBusy());
-    Unlink();
-    mValue.reset();
-    mTextCtrlElement = nullptr;
-  }
+  MOZ_CAN_RUN_SCRIPT void UnlinkInternal();
 
   void ValueWasChanged(bool aNotify);
 
   MOZ_CAN_RUN_SCRIPT void DestroyEditor();
   MOZ_CAN_RUN_SCRIPT void Clear();
 
   nsresult InitializeRootNode();