Bug 373586 , r+sr=roc

layout/forms/nsTextControlFrame.cpp

@@ -1031,7 +1031,6 @@ nsTextControlFrame::nsTextControlFrame(nsIPresShell* aShell, nsStyleContext* aCo
   , mDidPreDestroy(PR_FALSE)
   , mFireChangeEventState(PR_FALSE)
   , mTextListener(nsnull)
-  , mScrollableView(nsnull)
 #ifdef DEBUG
   , mCreateFrameForCalled(PR_FALSE)
 #endif
@@ -1125,7 +1124,10 @@ nsTextControlFrame::PreDestroy()
 
   mEditor = nsnull;
   mSelCon = nsnull;
-  mFrameSel = nsnull;
+  if (mFrameSel) {
+    mFrameSel->SetScrollableViewProvider(nsnull);
+    mFrameSel = nsnull;
+  }
 
 //unregister self from content
   mTextListener->SetFrame(nsnull);
@@ -1164,6 +1166,9 @@ nsTextControlFrame::Destroy()
   if (!mDidPreDestroy) {
     PreDestroy();
   }
+  if (mFrameSel) {
+    mFrameSel->SetScrollableViewProvider(nsnull);
+  }
   nsContentUtils::DestroyAnonymousContent(&mAnonymousDiv);
   nsBoxFrame::Destroy();
 }
@@ -1382,6 +1387,7 @@ nsTextControlFrame::CreateFrameFor(nsIContent*      aContent)
   mFrameSel = do_CreateInstance(kFrameSelectionCID, &rv);
   if (NS_FAILED(rv))
     return nsnull;
+  mFrameSel->SetScrollableViewProvider(this);
 
   // Create a SelectionController
 
@@ -1864,7 +1870,8 @@ nsresult nsTextControlFrame::SetFormProperty(nsIAtom* aName, const nsAString& aV
         // has changed.
         SetValueChanged(PR_TRUE);
       }
-      SetValue(aValue);   // set new text value
+      nsresult rv = SetValue(aValue); // set new text value
+      NS_ENSURE_SUCCESS(rv, rv);
     }
     else if (nsGkAtoms::select == aName)
     {
@@ -2566,13 +2573,15 @@ nsTextControlFrame::GetValue(nsAString& aValue, PRBool aIgnoreWrap) const
 
 // END IMPLEMENTING NS_IFORMCONTROLFRAME
 
-void
+nsresult
 nsTextControlFrame::SetValue(const nsAString& aValue)
 {
   // XXX this method should actually propagate errors!  It'd make debugging it
   // so much easier...
   if (mEditor && mUseEditor) 
   {
+    nsCOMPtr<nsIEditor> editor = mEditor;
+    nsWeakFrame weakFrame(this);
     nsAutoString currentValue;
     GetValue(currentValue, PR_FALSE);
     if (IsSingleLineTextControl())
@@ -2590,9 +2599,9 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
       ::PlatformToDOMLineBreaks(currentValue);
 
       nsCOMPtr<nsIDOMDocument>domDoc;
-      nsresult rv = mEditor->GetDocument(getter_AddRefs(domDoc));
-      if (NS_FAILED(rv)) return;
-      if (!domDoc) return;
+      nsresult rv = editor->GetDocument(getter_AddRefs(domDoc));
+      NS_ENSURE_SUCCESS(rv, rv);
+      NS_ENSURE_STATE(domDoc);
 
       // Time to mess with our security context... See comments in GetValue()
       // for why this is needed.  Note that we have to do this up here, because
@@ -2612,15 +2621,15 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
       }
 
       mSelCon->SelectAll();
-      nsCOMPtr<nsIPlaintextEditor> htmlEditor = do_QueryInterface(mEditor);
-      if (!htmlEditor) {
+      nsCOMPtr<nsIPlaintextEditor> plaintextEditor = do_QueryInterface(editor);
+      if (!plaintextEditor) {
         NS_WARNING("Somehow not a plaintext editor?");
         if (pushed) {
           JSContext* cx;
           stack->Pop(&cx);
           NS_ASSERTION(!cx, "Unexpected JSContext popped!");
         }
-        return;
+        return NS_ERROR_FAILURE;
       }
 
       // Since this code does not handle user-generated changes to the text,
@@ -2637,21 +2646,21 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
       // get the flags, remove readonly and disabled, set the value,
       // restore flags
       PRUint32 flags, savedFlags;
-      mEditor->GetFlags(&savedFlags);
+      editor->GetFlags(&savedFlags);
       flags = savedFlags;
       flags &= ~(nsIPlaintextEditor::eEditorDisabledMask);
       flags &= ~(nsIPlaintextEditor::eEditorReadonlyMask);
-      mEditor->SetFlags(flags);
+      editor->SetFlags(flags);
 
       if (currentValue.Length() < 1)
-        mEditor->DeleteSelection(nsIEditor::eNone);
+        editor->DeleteSelection(nsIEditor::eNone);
       else {
-        nsCOMPtr<nsIPlaintextEditor> textEditor = do_QueryInterface(mEditor);
+        nsCOMPtr<nsIPlaintextEditor> textEditor = do_QueryInterface(editor);
         if (textEditor)
           textEditor->InsertText(currentValue);
       }
 
-      mEditor->SetFlags(savedFlags);
+      editor->SetFlags(savedFlags);
       if (selPriv)
         selPriv->EndBatchChanges();
 
@@ -2661,6 +2670,7 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
         NS_ASSERTION(!cx, "Unexpected JSContext popped!");
       }
 
+      NS_ENSURE_STATE(weakFrame.IsAlive());
       if (outerTransaction)
         mNotifyOnInput = PR_TRUE;
 
@@ -2673,12 +2683,14 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
       }
     }
 
-    if (mScrollableView)
+    NS_ENSURE_STATE(weakFrame.IsAlive());
+    nsIScrollableView* scrollableView = GetScrollableView();
+    if (scrollableView)
     {
       // Scroll the upper left corner of the text control's
       // content area back into view.
 
-      mScrollableView->ScrollTo(0, 0, NS_VMREFRESH_NO_SYNC);
+      scrollableView->ScrollTo(0, 0, NS_VMREFRESH_NO_SYNC);
     }
   }
   else
@@ -2690,6 +2702,7 @@ nsTextControlFrame::SetValue(const nsAString& aValue)
       textControl->TakeTextFrameValue(aValue);
     }
   }
+  return NS_OK;
 }
 
 
@@ -2748,17 +2761,17 @@ nsTextControlFrame::SetInitialChildList(nsIAtom*        aListName,
                                       listener, PR_FALSE, systemGroup);
   }
 
-  if (scrollableFrame) {
-    mScrollableView = scrollableFrame->GetScrollableView();
-    mFrameSel->SetScrollableView(mScrollableView);
-  }
-
   return rv;
 }
 
 nsIScrollableView* nsTextControlFrame::GetScrollableView()
 {
-  return mScrollableView;
+  nsIFrame* first = GetFirstChild(nsnull);
+  nsIScrollableFrame* scrollableFrame = nsnull;
+  if (first) {
+    CallQueryInterface(first, &scrollableFrame);
+  }
+  return scrollableFrame ? scrollableFrame->GetScrollableView() : nsnull;
 }
 
 PRBool

layout/forms/nsTextControlFrame.h

@@ -121,7 +121,11 @@ public:
   virtual void PostCreateFrames();
 
   // Utility methods to set current widget state
-  void SetValue(const nsAString& aValue);
+
+  // Be careful when using this method.
+  // Calling it may cause |this| to be deleted.
+  // In that case the method returns an error value.
+  nsresult SetValue(const nsAString& aValue);
   NS_IMETHOD SetInitialChildList(nsIAtom*        aListName,
                                  nsIFrame*       aChildList);
 
@@ -292,8 +296,6 @@ private:
   nsCOMPtr<nsISelectionController> mSelCon;
   nsCOMPtr<nsFrameSelection> mFrameSel;
   nsTextInputListener* mTextListener;
-  // XXX This seems unsafe; what's keeping it around?
-  nsIScrollableView *mScrollableView;
   nsString mFocusedValue;
 
 #ifdef DEBUG

layout/generic/nsFrameSelection.h

@@ -41,7 +41,7 @@
 #include "nsIFrame.h"
 #include "nsIContent.h"
 #include "nsISelectionController.h"
-
+#include "nsIScrollableViewProvider.h"
 #include "nsITableLayout.h"
 #include "nsITableCellLayout.h"
 #include "nsIDOMElement.h"
@@ -209,14 +209,24 @@ public:
    */
   void Init(nsIPresShell *aShell, nsIContent *aLimiter);
 
-  /* SetScrollableView sets the scroll view
-   *  @param aScrollView is the scroll view for this selection.
+  /**
+   * SetScrollableViewProvider sets the scroll view provider.
+   * @param aProvider The provider of the scroll view.
    */
-  void SetScrollableView(nsIScrollableView *aScrollView) { mScrollView = aScrollView; }
+  void SetScrollableViewProvider(nsIScrollableViewProvider* aProvider)
+  {
+    mScrollableViewProvider = aProvider;
+  }
 
-  /* GetScrollableView gets the current scroll view
+  /**
+   * GetScrollableView returns the current scroll view.
    */
-  nsIScrollableView* GetScrollableView() { return mScrollView; }
+  nsIScrollableView* GetScrollableView()
+  {
+    return mScrollableViewProvider
+      ? mScrollableViewProvider->GetScrollableView()
+      : nsnull;
+  }
 
   /** HandleClick will take the focus to the new frame at the new offset and 
    *  will either extend the selection from the old anchor, or replace the old anchor.
@@ -632,7 +642,7 @@ private:
 #endif
 
   PRInt32 mDesiredX;
-  nsIScrollableView *mScrollView;
+  nsIScrollableViewProvider* mScrollableViewProvider;
 
   nsMouseEvent mDelayedMouseEvent;
 

layout/generic/nsSelection.cpp

@@ -1205,7 +1205,7 @@ nsFrameSelection::Init(nsIPresShell *aShell, nsIContent *aLimiter)
   mMouseDownState = PR_FALSE;
   mDesiredXSet = PR_FALSE;
   mLimiter = aLimiter;
-  mScrollView = nsnull;
+  mScrollableViewProvider = nsnull;
   mCaretMovementStyle = nsContentUtils::GetIntPref("bidi.edit.caret_movement_style", 2);
 }