Closed Bug 748216 Opened 12 years ago Closed 12 years ago

Avoid dynamic allocation of nsHTMLTextAreaElement::mState

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

Details

Attachments

(1 file, 1 obsolete file)

We currently allocate mState dynamically.  This is stupid, since this object will *always* need to have the mState initialized properly, and it's inefficient.

The reason that I did this was that I did not know how to traverse mState for cycle collection as a simple member variable.  Here's the related code: <http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsHTMLTextAreaElement.cpp#359>

The reason I did this was that I did not know how to properly traverse this member if it's not a pointer.  Doing the obvious (see below) seems to lead to cycle collector faults:

NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_PTR(&tmp->mState, nsTextEditorState, "mState")

Here's the stack for a sample cycle collector fault:

Fault in cycle collector: zero refcount (ptr: 0x127070ff0)
###!!! ABORT: cycle collector fault: file /Users/ehsanakhgari/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp, line 1252
Fault(char const*, void const*)+0x00000079 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F82339]
Fault(char const*, PtrInfo*)+0x00000020 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F82070]
GCGraphBuilder::DescribeRefCountedNode(unsigned int, unsigned long, char const*)+0x00000043 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F82383]
nsTextEditorState::cycleCollection::Traverse(void*, nsCycleCollectionTraversalCallback&)+0x00000058 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x00B135D8]
GCGraphBuilder::Traverse(PtrInfo*)+0x0000007A [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F81FFA]
nsCycleCollector::MarkRoots(GCGraphBuilder&)+0x00000079 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F83079]
nsCycleCollector::BeginCollection(nsICycleCollectorListener*)+0x0000019A [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F8439A]
nsCycleCollectorRunner::Run()+0x00000147 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F89A47]
nsThread::ProcessNextEvent(bool, bool*)+0x00000558 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F64A88]
NS_ProcessNextEvent_P(nsIThread*, bool)+0x0000009F [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01ED613F]
nsThread::ThreadFunc(void*)+0x00000127 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/XUL +0x01F63727]
_pt_root+0x00000163 [/Users/ehsanakhgari/moz/mozilla-central/obj-ff-dbg/dist/NightlyDebug.app/Contents/MacOS/libnspr4.dylib +0x00038B93]
_pthread_start+0x0000014B [/usr/lib/libSystem.B.dylib +0x00039FD6]
###!!! ABORT: cycle collector fault: file /Users/ehsanakhgari/moz/mozilla-central/xpcom/base/nsCycleCollector.cpp, line 1252

Can someone please tell me how to do this properly?
It looks like the nsTextEditorState is reporting to the CC that its ref count is 0.  It gets its refcount like this: |tmp->mRefCnt.get()|.  Are you failing to add ref mState somehow?  What are you changing its type to?
Ah, right, you are putting it inline.  That is a little weird.  I'm not sure what the right way to deal with that is.  Maybe you can manually addref it?
nsHTMLTextAreaElement holds the only reference to nsTextEditorState.  Kyle told me on IRC that I should add a Traverse method to nsTextEditorState (like nsINode::Traverse) and just call that directly.  That requires me to not be sleepy, so I'll try that tomorrow.
Ah, yes, that sounds right.  nsBidiUtils or something like that does that.
Don't use refcounted classes as inline or stack variables.
Attached patch WIP (obsolete) — Splinter Review
This is the patch I have so far, with several failed attempts at getting Traverse right commented out.  Nothing has worked for me so far.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #617892 - Flags: feedback?(khuey)
What you need to do is define a new method nsTextEditorState::Traverse( nsCycleCollectionTraversalCallback &cb).  The body of this function is the same as the current CC traverse function, which you should make invoke the new Traverse function.  The problem you are having is that the CC traverse function invokes a callback describing the object, but you don't want to do that, you just want to report the children.
(then call this new method from nsHTMLTextAreaElement's CC traverse function)
Attached patch Patch (v1)Splinter Review
Attachment #617892 - Attachment is obsolete: true
Attachment #617892 - Flags: feedback?(khuey)
Attachment #617934 - Flags: review?(continuation)
Attachment #617934 - Flags: review?(bugs)
Comment on attachment 617934 [details] [diff] [review]
Patch (v1)

>commit f6b98bcb1e8400150e9f3512b8500e5bcd2b5531
>Author: Ehsan Akhgari <ehsan.akhgari@gmail.com>
>Date:   Tue Apr 24 00:19:26 2012 -0400
>
>    Bug 748216 - Avoid dynamic allocation of nsHTMLTextAreaElement::mState
>
>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>index 843b9c0..c223c9e 100644
>--- a/content/html/content/src/nsHTMLInputElement.cpp
>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>@@ -573,8 +573,7 @@ nsHTMLInputElement::nsHTMLInputElement(already_AddRefed<nsINodeInfo> aNodeInfo,
>   , mCanShowInvalidUI(true)
> {
>   mInputData.mState = new nsTextEditorState(this);
>-  NS_ADDREF(mInputData.mState);
>-  
>+
>   if (!gUploadLastDir)
>     nsHTMLInputElement::InitUploadLastDir();
> 
>@@ -605,7 +604,8 @@ nsHTMLInputElement::FreeData()
>     mInputData.mValue = nsnull;
>   } else {
>     UnbindFromFrame(nsnull);
>-    NS_IF_RELEASE(mInputData.mState);
>+    delete mInputData.mState;
>+    mInputData.mState = nsnull;
>   }
> }
> 
>@@ -630,7 +630,7 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLInputElement,
>                                                   nsGenericHTMLFormElement)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mControllers)
>   if (tmp->IsSingleLineTextControl(false)) {
>-    NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mInputData.mState, nsTextEditorState)
>+    tmp->mInputData.mState->Traverse(cb);
>   }
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMARRAY(mFiles)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mFileList)
>@@ -644,6 +644,9 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLInputElement,
>     tmp->mFileList->Disconnect();
>     tmp->mFileList = nsnull;
>   }
>+  if (tmp->IsSingleLineTextControl(false)) {
>+    tmp->mInputData.mState->Unlink();
>+  }
>   //XXX should unlink more?
> NS_IMPL_CYCLE_COLLECTION_UNLINK_END
>                                                               
>@@ -2396,7 +2399,6 @@ nsHTMLInputElement::HandleTypeChange(PRUint8 aNewType)
>   if (isNewTypeSingleLine && !isCurrentTypeSingleLine) {
>     FreeData();
>     mInputData.mState = new nsTextEditorState(this);
>-    NS_ADDREF(mInputData.mState);
>   } else if (isCurrentTypeSingleLine && !isNewTypeSingleLine) {
>     FreeData();
>   }
>diff --git a/content/html/content/src/nsHTMLTextAreaElement.cpp b/content/html/content/src/nsHTMLTextAreaElement.cpp
>index 39157e4..76505af 100644
>--- a/content/html/content/src/nsHTMLTextAreaElement.cpp
>+++ b/content/html/content/src/nsHTMLTextAreaElement.cpp
>@@ -259,7 +259,7 @@ protected:
>   bool                     mCanShowValidUI;
> 
>   /** The state of the text editor (selection controller and the editor) **/
>-  nsRefPtr<nsTextEditorState> mState;
>+  nsTextEditorState mState;
> 
>   NS_IMETHOD SelectAll(nsPresContext* aPresContext);
>   /**
>@@ -333,7 +333,7 @@ nsHTMLTextAreaElement::nsHTMLTextAreaElement(already_AddRefed<nsINodeInfo> aNode
>     mDisabledChanged(false),
>     mCanShowInvalidUI(true),
>     mCanShowValidUI(true),
>-    mState(new nsTextEditorState(this))
>+    mState(this)
> {
>   AddMutationObserver(this);
> 
>@@ -352,11 +352,12 @@ NS_IMPL_CYCLE_COLLECTION_CLASS(nsHTMLTextAreaElement)
> NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLTextAreaElement,
>                                                 nsGenericHTMLFormElement)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mControllers)
>+  tmp->mState.Unlink();
> NS_IMPL_CYCLE_COLLECTION_UNLINK_END
> NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsHTMLTextAreaElement,
>                                                   nsGenericHTMLFormElement)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mControllers)
>-  NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_MEMBER(mState, nsTextEditorState)
>+  tmp->mState.Traverse(cb);
> NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
> 
> NS_IMPL_ADDREF_INHERITED(nsHTMLTextAreaElement, nsGenericElement) 
>@@ -498,76 +499,76 @@ nsHTMLTextAreaElement::GetValue(nsAString& aValue)
> void
> nsHTMLTextAreaElement::GetValueInternal(nsAString& aValue, bool aIgnoreWrap) const
> {
>-  mState->GetValue(aValue, aIgnoreWrap);
>+  mState.GetValue(aValue, aIgnoreWrap);
> }
> 
> NS_IMETHODIMP_(nsIEditor*)
> nsHTMLTextAreaElement::GetTextEditor()
> {
>-  return mState->GetEditor();
>+  return mState.GetEditor();
> }
> 
> NS_IMETHODIMP_(nsISelectionController*)
> nsHTMLTextAreaElement::GetSelectionController()
> {
>-  return mState->GetSelectionController();
>+  return mState.GetSelectionController();
> }
> 
> NS_IMETHODIMP_(nsFrameSelection*)
> nsHTMLTextAreaElement::GetConstFrameSelection()
> {
>-  return mState->GetConstFrameSelection();
>+  return mState.GetConstFrameSelection();
> }
> 
> NS_IMETHODIMP
> nsHTMLTextAreaElement::BindToFrame(nsTextControlFrame* aFrame)
> {
>-  return mState->BindToFrame(aFrame);
>+  return mState.BindToFrame(aFrame);
> }
> 
> NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::UnbindFromFrame(nsTextControlFrame* aFrame)
> {
>   if (aFrame) {
>-    mState->UnbindFromFrame(aFrame);
>+    mState.UnbindFromFrame(aFrame);
>   }
> }
> 
> NS_IMETHODIMP
> nsHTMLTextAreaElement::CreateEditor()
> {
>-  return mState->PrepareEditor();
>+  return mState.PrepareEditor();
> }
> 
> NS_IMETHODIMP_(nsIContent*)
> nsHTMLTextAreaElement::GetRootEditorNode()
> {
>-  return mState->GetRootNode();
>+  return mState.GetRootNode();
> }
> 
> NS_IMETHODIMP_(nsIContent*)
> nsHTMLTextAreaElement::CreatePlaceholderNode()
> {
>-  NS_ENSURE_SUCCESS(mState->CreatePlaceholderNode(), nsnull);
>-  return mState->GetPlaceholderNode();
>+  NS_ENSURE_SUCCESS(mState.CreatePlaceholderNode(), nsnull);
>+  return mState.GetPlaceholderNode();
> }
> 
> NS_IMETHODIMP_(nsIContent*)
> nsHTMLTextAreaElement::GetPlaceholderNode()
> {
>-  return mState->GetPlaceholderNode();
>+  return mState.GetPlaceholderNode();
> }
> 
> NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::UpdatePlaceholderText(bool aNotify)
> {
>-  mState->UpdatePlaceholderText(aNotify);
>+  mState.UpdatePlaceholderText(aNotify);
> }
> 
> NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::SetPlaceholderClass(bool aVisible, bool aNotify)
> {
>-  mState->SetPlaceholderClass(aVisible, aNotify);
>+  mState.SetPlaceholderClass(aVisible, aNotify);
> }
> 
> nsresult
>@@ -578,7 +579,7 @@ nsHTMLTextAreaElement::SetValueInternal(const nsAString& aValue,
>   // nsTextControlFrame::UpdateValueDisplay retrieves the correct value
>   // if needed.
>   SetValueChanged(true);
>-  mState->SetValue(aValue, aUserInput);
>+  mState.SetValue(aValue, aUserInput);
> 
>   return NS_OK;
> }
>@@ -605,8 +606,8 @@ nsHTMLTextAreaElement::SetValueChanged(bool aValueChanged)
>   bool previousValue = mValueChanged;
> 
>   mValueChanged = aValueChanged;
>-  if (!aValueChanged && !mState->IsEmpty()) {
>-    mState->EmptyValue();
>+  if (!aValueChanged && !mState.IsEmpty()) {
>+    mState.EmptyValue();
>   }
> 
>   if (mValueChanged != previousValue) {
>@@ -848,8 +849,8 @@ nsHTMLTextAreaElement::GetSelectionStart(PRInt32 *aSelectionStart)
>   PRInt32 selEnd;
>   nsresult rv = GetSelectionRange(aSelectionStart, &selEnd);
> 
>-  if (NS_FAILED(rv) && mState->IsSelectionCached()) {
>-    *aSelectionStart = mState->GetSelectionProperties().mStart;
>+  if (NS_FAILED(rv) && mState.IsSelectionCached()) {
>+    *aSelectionStart = mState.GetSelectionProperties().mStart;
>     return NS_OK;
>   }
>   return rv;
>@@ -858,8 +859,8 @@ nsHTMLTextAreaElement::GetSelectionStart(PRInt32 *aSelectionStart)
> NS_IMETHODIMP
> nsHTMLTextAreaElement::SetSelectionStart(PRInt32 aSelectionStart)
> {
>-  if (mState->IsSelectionCached()) {
>-    mState->GetSelectionProperties().mStart = aSelectionStart;
>+  if (mState.IsSelectionCached()) {
>+    mState.GetSelectionProperties().mStart = aSelectionStart;
>     return NS_OK;
>   }
> 
>@@ -884,8 +885,8 @@ nsHTMLTextAreaElement::GetSelectionEnd(PRInt32 *aSelectionEnd)
>   PRInt32 selStart;
>   nsresult rv = GetSelectionRange(&selStart, aSelectionEnd);
> 
>-  if (NS_FAILED(rv) && mState->IsSelectionCached()) {
>-    *aSelectionEnd = mState->GetSelectionProperties().mEnd;
>+  if (NS_FAILED(rv) && mState.IsSelectionCached()) {
>+    *aSelectionEnd = mState.GetSelectionProperties().mEnd;
>     return NS_OK;
>   }
>   return rv;
>@@ -894,8 +895,8 @@ nsHTMLTextAreaElement::GetSelectionEnd(PRInt32 *aSelectionEnd)
> NS_IMETHODIMP
> nsHTMLTextAreaElement::SetSelectionEnd(PRInt32 aSelectionEnd)
> {
>-  if (mState->IsSelectionCached()) {
>-    mState->GetSelectionProperties().mEnd = aSelectionEnd;
>+  if (mState.IsSelectionCached()) {
>+    mState.GetSelectionProperties().mEnd = aSelectionEnd;
>     return NS_OK;
>   }
> 
>@@ -960,8 +961,8 @@ nsHTMLTextAreaElement::GetSelectionDirection(nsAString& aDirection)
>   }
> 
>   if (NS_FAILED(rv)) {
>-    if (mState->IsSelectionCached()) {
>-      DirectionToName(mState->GetSelectionProperties().mDirection, aDirection);
>+    if (mState.IsSelectionCached()) {
>+      DirectionToName(mState.GetSelectionProperties().mDirection, aDirection);
>       return NS_OK;
>     }
>   }
>@@ -971,14 +972,14 @@ nsHTMLTextAreaElement::GetSelectionDirection(nsAString& aDirection)
> 
> NS_IMETHODIMP
> nsHTMLTextAreaElement::SetSelectionDirection(const nsAString& aDirection) {
>-  if (mState->IsSelectionCached()) {
>+  if (mState.IsSelectionCached()) {
>     nsITextControlFrame::SelectionDirection dir = nsITextControlFrame::eNone;
>     if (aDirection.EqualsLiteral("forward")) {
>       dir = nsITextControlFrame::eForward;
>     } else if (aDirection.EqualsLiteral("backward")) {
>       dir = nsITextControlFrame::eBackward;
>     }
>-    mState->GetSelectionProperties().mDirection = dir;
>+    mState.GetSelectionProperties().mDirection = dir;
>     return NS_OK;
>   }
> 
>@@ -1293,7 +1294,7 @@ nsHTMLTextAreaElement::AfterSetAttr(PRInt32 aNameSpaceID, nsIAtom* aName,
> 
>     if (aName == nsGkAtoms::readonly) {
>       UpdateEditableState(aNotify);
>-      mState->UpdateEditableState(aNotify);
>+      mState.UpdateEditableState(aNotify);
>     }
>     UpdateState(aNotify);
>   }
>@@ -1525,20 +1526,20 @@ NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::GetTextEditorValue(nsAString& aValue,
>                                           bool aIgnoreWrap) const
> {
>-  mState->GetValue(aValue, aIgnoreWrap);
>+  mState.GetValue(aValue, aIgnoreWrap);
> }
> 
> NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::SetTextEditorValue(const nsAString& aValue,
>                                           bool aUserInput)
> {
>-  mState->SetValue(aValue, aUserInput);
>+  mState.SetValue(aValue, aUserInput);
> }
> 
> NS_IMETHODIMP_(void)
> nsHTMLTextAreaElement::InitializeKeyboardEventListeners()
> {
>-  mState->InitializeKeyboardEventListeners();
>+  mState.InitializeKeyboardEventListeners();
> }
> 
> NS_IMETHODIMP_(void)
>@@ -1559,7 +1560,7 @@ nsHTMLTextAreaElement::OnValueChanged(bool aNotify)
> NS_IMETHODIMP_(bool)
> nsHTMLTextAreaElement::HasCachedSelection()
> {
>-  return mState->IsSelectionCached();
>+  return mState.IsSelectionCached();
> }
> 
> void
>diff --git a/content/html/content/src/nsTextEditorState.cpp b/content/html/content/src/nsTextEditorState.cpp
>index 1371e15..d6f3ced 100644
>--- a/content/html/content/src/nsTextEditorState.cpp
>+++ b/content/html/content/src/nsTextEditorState.cpp
>@@ -970,22 +970,24 @@ nsTextEditorState::Clear()
>   NS_IF_RELEASE(mTextListener);
> }
> 
>-NS_IMPL_CYCLE_COLLECTION_CLASS(nsTextEditorState)
>-NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(nsTextEditorState, AddRef)
>-NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(nsTextEditorState, Release)
>-NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_NATIVE(nsTextEditorState)
>+void nsTextEditorState::Unlink()
>+{
>+  nsTextEditorState* tmp = this;
>   tmp->Clear();
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mSelCon)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mEditor)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mRootNode)
>   NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mPlaceholderDiv)
>-NS_IMPL_CYCLE_COLLECTION_UNLINK_END
>-NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NATIVE_BEGIN(nsTextEditorState)
>+}
>+
>+void nsTextEditorState::Traverse(nsCycleCollectionTraversalCallback& cb)
>+{
>+  nsTextEditorState* tmp = this;
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR_AMBIGUOUS(mSelCon, nsISelectionController)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mEditor)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mRootNode)
>   NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR(mPlaceholderDiv)
>-NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
>+}
> 
> nsFrameSelection*
> nsTextEditorState::GetConstFrameSelection() {
>diff --git a/content/html/content/src/nsTextEditorState.h b/content/html/content/src/nsTextEditorState.h
>index c16fff2..56813a8 100644
>--- a/content/html/content/src/nsTextEditorState.h
>+++ b/content/html/content/src/nsTextEditorState.h
>@@ -154,8 +154,8 @@ public:
>   explicit nsTextEditorState(nsITextControlElement* aOwningElement);
>   ~nsTextEditorState();
> 
>-  NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(nsTextEditorState)
>-  NS_INLINE_DECL_REFCOUNTING(nsTextEditorState)
>+  void Traverse(nsCycleCollectionTraversalCallback& cb);
>+  void Unlink();
> 
>   nsIEditor* GetEditor();
>   nsISelectionController* GetSelectionController() const;
Attachment #617934 - Flags: review?(bugs) → review+
Attachment #617934 - Flags: review?(continuation)
Comment on attachment 617934 [details] [diff] [review]
Patch (v1)

Review of attachment 617934 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLInputElement.cpp
@@ +604,5 @@
>      mInputData.mValue = nsnull;
>    } else {
>      UnbindFromFrame(nsnull);
> +    delete mInputData.mState;
> +    mInputData.mState = nsnull;

Can you really delete an inline struct? And then assign null to it?
(In reply to Andrew McCreight [:mccr8] from comment #12)
> Comment on attachment 617934 [details] [diff] [review]
> Patch (v1)
> 
> Review of attachment 617934 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/html/content/src/nsHTMLInputElement.cpp
> @@ +604,5 @@
> >      mInputData.mValue = nsnull;
> >    } else {
> >      UnbindFromFrame(nsnull);
> > +    delete mInputData.mState;
> > +    mInputData.mState = nsnull;
> 
> Can you really delete an inline struct? And then assign null to it?

mInputData.mState is a nsTextEditorState*.  :-)

Here's the reason for this craziness.  <input> elements can be text boxes, *and* other things, and <textarea> elements can only be text boxes.  So, we need to have a dynamic nsTextEditorState for nsHTMLInputElement, which only exists when the <input> is actually a text box (and thanks to C++, we can only use a pointer for that), but for nsHTMLTextAreaElement, we should always have an associated nsTextEditorState, which is what this bug is about.
Oh, I see.  Thanks for the explanation!
Try run for eec25295ecd7 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=eec25295ecd7
Results (out of 221 total builds):
    success: 177
    warnings: 39
    failure: 2
    other: 3
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/eakhgari@mozilla.com-eec25295ecd7
 Timed out after 12 hours without completing.
https://hg.mozilla.org/mozilla-central/rev/6ac410348fac
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: