Bug 376662. Convert nsIFrame::GetOffsetTo to not use views. We need to ensure that for popups and scrollframes, views and frames are kept in sync at *all* times. Also fixes bugs in tests for NS_FRAME_NO_MOVE_FRAME. r+sr=bzbarsky

accessible/src/base/nsDocAccessible.h

@@ -88,6 +88,7 @@ class nsDocAccessible : public nsHyperTextAccessibleWrap,
 
     // ----- nsIScrollPositionListener ---------------------------
     NS_IMETHOD ScrollPositionWillChange(nsIScrollableView *aView, nscoord aX, nscoord aY);
+    virtual void ViewPositionDidChange(nsIScrollableView* aScrollable) {}
     NS_IMETHOD ScrollPositionDidChange(nsIScrollableView *aView, nscoord aX, nscoord aY);
 
     // nsIDocumentObserver

content/base/src/nsGenericElement.cpp

@@ -1000,7 +1000,8 @@ nsGenericElement::GetOffsetRect(nsRect& aRect, nsIContent** aOffsetParent)
   // It doesn't really matter what we use as aRelativeTo here, since
   // we only care about the size. Using 'parent' might make things
   // a bit faster by speeding up the internal GetOffsetTo operations.
-  nsRect rcFrame = nsLayoutUtils::GetAllInFlowRectsUnion(frame, nsnull);
+  nsIFrame* parent = frame->GetParent() ? frame->GetParent() : frame;
+  nsRect rcFrame = nsLayoutUtils::GetAllInFlowRectsUnion(frame, parent);
   aRect.width = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.width);
   aRect.height = nsPresContext::AppUnitsToIntCSSPixels(rcFrame.height);
 }

layout/base/nsLayoutUtils.cpp

@@ -264,7 +264,8 @@ nsLayoutUtils::IsGeneratedContentFor(nsIContent* aContent,
 
 // static
 nsIFrame*
-nsLayoutUtils::GetCrossDocParentFrame(nsIFrame* aFrame)
+nsLayoutUtils::GetCrossDocParentFrame(const nsIFrame* aFrame,
+                                      nsPoint* aExtraOffset)
 {
   nsIFrame* p = aFrame->GetParent();
   if (p)
@@ -276,6 +277,9 @@ nsLayoutUtils::GetCrossDocParentFrame(nsIFrame* aFrame)
   v = v->GetParent(); // anonymous inner view
   if (!v)
     return nsnull;
+  if (aExtraOffset) {
+    *aExtraOffset += v->GetPosition();
+  }
   v = v->GetParent(); // subdocumentframe's view
   if (!v)
     return nsnull;

layout/base/nsLayoutUtils.h

@@ -229,8 +229,11 @@ public:
    * Get the parent of aFrame. If aFrame is the root frame for a document,
    * and the document has a parent document in the same view hierarchy, then
    * we try to return the subdocumentframe in the parent document.
+   * @param aExtraOffset [in/out] if non-null, then as we cross documents
+   * an extra offset may be required and it will be added to aCrossDocOffset
    */
-  static nsIFrame* GetCrossDocParentFrame(nsIFrame* aFrame);
+  static nsIFrame* GetCrossDocParentFrame(const nsIFrame* aFrame,
+                                          nsPoint* aCrossDocOffset = nsnull);
   
   /**
    * IsProperAncestorFrame checks whether aAncestorFrame is an ancestor

layout/forms/nsComboboxControlFrame.cpp

@@ -479,7 +479,7 @@ nsComboboxControlFrame::ReflowDropdown(nsPresContext*  aPresContext,
   
   // Allow the child to move/size/change-visibility its view if it's currently
   // dropped down
-  PRInt32 flags = NS_FRAME_NO_MOVE_VIEW | NS_FRAME_NO_VISIBILITY | NS_FRAME_NO_SIZE_VIEW;
+  PRInt32 flags = NS_FRAME_NO_MOVE_FRAME | NS_FRAME_NO_VISIBILITY | NS_FRAME_NO_SIZE_VIEW;
   if (mDroppedDown) {
     flags = 0;
   }

layout/generic/nsContainerFrame.cpp

@@ -764,7 +764,7 @@ nsContainerFrame::ReflowChild(nsIFrame*                aKidFrame,
   // and its view if requested
   aKidFrame->WillReflow(aPresContext);
 
-  if (0 == (aFlags & NS_FRAME_NO_MOVE_FRAME)) {
+  if (NS_FRAME_NO_MOVE_FRAME != (aFlags & NS_FRAME_NO_MOVE_FRAME)) {
     if ((aFlags & NS_FRAME_INVALIDATE_ON_MOVE) &&
         !(aKidFrame->GetStateBits() & NS_FRAME_FIRST_REFLOW) &&
         aKidFrame->GetPosition() != nsPoint(aX, aY)) {

layout/generic/nsFrame.cpp

@@ -2154,7 +2154,7 @@ NS_IMETHODIMP nsFrame::HandleDrag(nsPresContext* aPresContext,
   PRUint8 selectStyle;
   IsSelectable(&selectable, &selectStyle);
   // XXX Do we really need to exclude non-selectable content here?
-  // GetContentAndOffsetsFromPoint can handle it just fine, although some
+  // GetContentOffsetsFromPoint can handle it just fine, although some
   // other stuff might not like it.
   if (!selectable)
     return NS_OK;
@@ -3452,20 +3452,23 @@ nsPoint nsIFrame::GetOffsetTo(const nsIFrame* aOther) const
 {
   NS_PRECONDITION(aOther,
                   "Must have frame for destination coordinate system!");
-  // Note that if we hit a view while walking up the frame tree we need to stop
-  // and switch to traversing the view tree so that we will deal with scroll
-  // views properly.
   nsPoint offset(0, 0);
   const nsIFrame* f;
-  for (f = this; !f->HasView() && f != aOther; f = f->GetParent()) {
+  for (f = this; f != aOther && f;
+       f = nsLayoutUtils::GetCrossDocParentFrame(f, &offset)) {
     offset += f->GetPosition();
   }
 
   if (f != aOther) {
-    // We found a view.  Switch to the view tree
-    nsPoint toViewOffset(0, 0);
-    nsIView* otherView = aOther->GetClosestView(&toViewOffset);
-    offset += f->GetView()->GetOffsetTo(otherView) - toViewOffset;
+    // Looks like aOther wasn't an ancestor of |this|.  So now we have
+    // the root-document-relative position of |this| in |offset|.  Convert back
+    // to the coordinates of aOther
+    nsPoint negativeOffset(0,0);
+    while (aOther) {
+      offset -= aOther->GetPosition();
+      aOther = nsLayoutUtils::GetCrossDocParentFrame(aOther, &negativeOffset);
+    }
+    offset -= negativeOffset;
   }
 
   return offset;
@@ -3534,88 +3537,6 @@ NS_IMETHODIMP nsFrame::GetOffsetFromView(nsPoint&  aOffset,
   return NS_OK;
 }
 
-// The (x,y) value of the frame's upper left corner is always
-// relative to its parentFrame's upper left corner, unless
-// its parentFrame has a view associated with it, in which case, it
-// will be relative to the upper left corner of the view returned
-// by a call to parentFrame->GetView().
-//
-// This means that while drilling down the frame hierarchy, from
-// parent to child frame, we sometimes need to take into account
-// crossing these view boundaries, because the coordinate system
-// changes from parent frame coordinate system, to the associated
-// view's coordinate system.
-//
-// GetOriginToViewOffset() is a utility method that returns the
-// offset necessary to map a point, relative to the frame's upper
-// left corner, into the coordinate system of the view associated
-// with the frame.
-//
-// If there is no view associated with the frame, or the view is
-// not a descendant of the frame's parent view (ex: scrolling popup menu),
-// the offset returned will be (0,0).
-
-NS_IMETHODIMP nsFrame::GetOriginToViewOffset(nsPoint&        aOffset,
-                                             nsIView**       aView) const
-{
-  nsresult rv = NS_OK;
-
-  aOffset.MoveTo(0,0);
-
-  if (aView)
-    *aView = nsnull;
-
-  if (HasView()) {
-    nsIView *view = GetView();
-    nsIView *parentView = nsnull;
-    nsPoint offsetToParentView;
-    rv = GetOffsetFromView(offsetToParentView, &parentView);
-
-    if (NS_SUCCEEDED(rv)) {
-      nsPoint viewOffsetFromParent(0,0);
-      nsIView *pview = view;
-
-      nsIViewManager* vVM = view->GetViewManager();
-
-      while (pview && pview != parentView) {
-        viewOffsetFromParent += pview->GetPosition();
-
-        nsIView *tmpView = pview->GetParent();
-        if (tmpView && vVM != tmpView->GetViewManager()) {
-          // Don't cross ViewManager boundaries!
-          // XXXbz why not?
-          break;
-        }
-        pview = tmpView;
-      }
-
-#ifdef DEBUG_KIN
-      if (pview != parentView) {
-        // XXX: At this point, pview is probably null since it traversed
-        //      all the way up view's parent hierarchy and did not run across
-        //      parentView. In the future, instead of just returning an offset
-        //      of (0,0) for this case, we may want offsetToParentView to
-        //      include the offset from the parentView to the top of the
-        //      view hierarchy which would make both offsetToParentView and
-        //      viewOffsetFromParent, offsets to the global coordinate space.
-        //      We'd have to investigate any perf impact this would have before
-        //      checking in such a change, so for now we just return (0,0).
-        //      -- kin    
-        NS_WARNING("view is not a descendant of parentView!");
-      }
-#endif // DEBUG
-
-      if (pview == parentView)
-        aOffset = offsetToParentView - viewOffsetFromParent;
-
-      if (aView)
-        *aView = view;
-    }
-  }
-
-  return rv;
-}
-
 /* virtual */ PRBool
 nsIFrame::AreAncestorViewsVisible() const
 {

layout/generic/nsFrame.h

@@ -228,7 +228,6 @@ public:
   virtual nsIFrame* GetNextInFlowVirtual() const;
   NS_IMETHOD  SetNextInFlow(nsIFrame*);
   NS_IMETHOD  GetOffsetFromView(nsPoint& aOffset, nsIView** aView) const;
-  NS_IMETHOD  GetOriginToViewOffset(nsPoint& aOffset, nsIView **aView) const;
   virtual nsIAtom* GetType() const;
   virtual PRBool IsContainingBlock() const;
 #ifdef NS_DEBUG

layout/generic/nsGfxScrollFrame.cpp

@@ -1793,6 +1793,14 @@ nsGfxScrollFrameInner::InternalScrollPositionDidChange(nscoord aX, nscoord aY)
                       aX - GetScrolledRect(GetScrollPortSize()).x);
 }
 
+void
+nsGfxScrollFrameInner::ViewPositionDidChange(nsIScrollableView* aScrollable)
+{
+  // Update frame position to match view offsets
+  nsPoint childOffset = mScrolledFrame->GetView()->GetOffsetTo(mOuter->GetView());
+  mScrolledFrame->SetPosition(childOffset);
+}
+
 /**
  * Called whenever actual scrolling happens for any reason.
  */
@@ -1801,10 +1809,6 @@ nsGfxScrollFrameInner::ScrollPositionDidChange(nsIScrollableView* aScrollable, n
 {
   NS_ASSERTION(!mViewInitiatedScroll, "Cannot reenter ScrollPositionDidChange");
 
-  // Update frame position to match view offsets
-  nsPoint childOffset = mScrolledFrame->GetView()->GetOffsetTo(mOuter->GetView());
-  mScrolledFrame->SetPosition(childOffset);
-
   mViewInitiatedScroll = PR_TRUE;
   InternalScrollPositionDidChange(aX, aY);
   mViewInitiatedScroll = PR_FALSE;

layout/generic/nsGfxScrollFrame.h

@@ -99,6 +99,7 @@ public:
   // nsIScrollPositionListener
 
   NS_IMETHOD ScrollPositionWillChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY);
+  virtual void ViewPositionDidChange(nsIScrollableView* aScrollable);
   NS_IMETHOD ScrollPositionDidChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY);
 
   // This gets called when the 'curpos' attribute on one of the scrollbars changes

layout/generic/nsHTMLFrame.cpp

@@ -115,6 +115,7 @@ public:
 
   // nsIScrollPositionListener
   NS_IMETHOD ScrollPositionWillChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY);
+  virtual void ViewPositionDidChange(nsIScrollableView* aScrollable) {}
   NS_IMETHOD ScrollPositionDidChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY);
 
   // nsICanvasFrame

layout/generic/nsIFrame.h

@@ -104,10 +104,10 @@ struct nsMargin;
 typedef class nsIFrame nsIBox;
 
 // IID for the nsIFrame interface
-// 04a7dee5-3435-47dc-bd42-a36c0f66a42c
+// 98a0c040-09cf-408b-b55f-321b4f8d9d67
 #define NS_IFRAME_IID \
-{ 0x04a7dee5, 0x3435, 0x47dc, \
-  { 0xbd, 0x42, 0xa3, 0x6c, 0x0f, 0x66, 0xa4, 0x2c } }
+    { 0x98a0c040, 0x09cf, 0x408b, \
+      { 0xb5, 0x5f, 0x32, 0x1b, 0x4f, 0x8d, 0x9d, 0x67 } }
 
 /**
  * Indication of how the frame can be split. This is used when doing runaround
@@ -1529,17 +1529,6 @@ public:
   NS_IMETHOD  GetOffsetFromView(nsPoint&  aOffset,
                                 nsIView** aView) const = 0;
 
-  /**
-   * Returns the offset from this frame's upper left corner to the upper
-   * left corner of the view returned by a call to GetView(). aOffset
-   * will contain the offset to the view or (0,0) if the frame has no
-   * view. aView will contain a pointer to the view returned by GetView().
-   * aView is optional, that is, you may pass null if you are not interested
-   * in getting a pointer to the view.
-   */
-  NS_IMETHOD  GetOriginToViewOffset(nsPoint&        aOffset,
-                                    nsIView**       aView) const = 0;
-
   /**
    * Returns true if and only if all views, from |GetClosestView| up to
    * the top of the view hierarchy are visible.

layout/generic/nsObjectFrame.cpp

@@ -386,6 +386,7 @@ public:
 
   // nsIScrollPositionListener interface
   NS_IMETHOD ScrollPositionWillChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY);
+  virtual void ViewPositionDidChange(nsIScrollableView* aScrollable) {}
   NS_IMETHOD ScrollPositionDidChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY);
 
   //locals

layout/xul/base/src/nsBox.cpp

@@ -274,7 +274,7 @@ nsBox::SetBounds(nsBoxLayoutState& aState, const nsRect& aRect, PRBool aRemoveOv
 
     flags |= stateFlags;
 
-    if (flags & NS_FRAME_NO_MOVE_FRAME)
+    if ((flags & NS_FRAME_NO_MOVE_FRAME) == NS_FRAME_NO_MOVE_FRAME)
       SetSize(nsSize(aRect.width, aRect.height));
     else
       SetRect(aRect);

layout/xul/base/src/nsMenuPopupFrame.cpp

@@ -885,6 +885,9 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
 
   nsPresContext* presContext = PresContext();
   nsIFrame* rootFrame = presContext->PresShell()->FrameManager()->GetRootFrame();
+  NS_ASSERTION(rootFrame->GetView() && GetView() &&
+               rootFrame->GetView() == GetView()->GetParent(),
+               "rootFrame's view is not our view's parent???");
 
   // if the frame is not specified, use the anchor node passed to ShowPopup. If
   // that wasn't specified either, use the root frame. Note that mAnchorContent
@@ -1237,11 +1240,9 @@ nsMenuPopupFrame::SetPopupPosition(nsIFrame* aAnchorFrame)
   presContext->GetViewManager()->MoveViewTo(GetView(), xpos, ypos); 
 
   // Now that we've positioned the view, sync up the frame's origin.
-  nsPoint frameOrigin = GetPosition();
-  nsPoint offsetToView;
-  GetOriginToViewOffset(offsetToView, nsnull);
-  frameOrigin -= offsetToView;
-  nsBoxFrame::SetPosition(frameOrigin);
+  // Note that (xpos,ypos) is the position relative to rootFrame.
+  nsBoxFrame::SetPosition(nsPoint(xpos, ypos) -
+                          GetParent()->GetOffsetTo(rootFrame));
 
   if (sizedToPopup) {
     nsBoxLayoutState state(PresContext());

view/public/nsIScrollPositionListener.h

@@ -47,9 +47,11 @@
 class nsIScrollableView;
 
 // IID for the nsIScrollPositionListener interface
-// {f8dfc500-6ad1-11d3-8360-a3f373ff79fc}
+// {98a0c040-09cf-408b-b55f-321b4f8d9d67}
+
 #define NS_ISCROLLPOSITIONLISTENER_IID \
-{ 0xf8dfc500, 0x6ad1, 0x11d3, { 0x83, 0x60, 0xa3, 0xf3, 0x73, 0xff, 0x79, 0xfc } }
+    { 0x98a0c040, 0x09cf, 0x408b, \
+      { 0xb5, 0x5f, 0x32, 0x1b, 0x4f, 0x8d, 0x9d, 0x67 } }
 
 /**
  * Provides a way for a client of an nsIScrollableView to learn about scroll position
@@ -60,6 +62,7 @@ public:
 	NS_DECLARE_STATIC_IID_ACCESSOR(NS_ISCROLLPOSITIONLISTENER_IID)
 
 	NS_IMETHOD ScrollPositionWillChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY) = 0;
+	virtual void ViewPositionDidChange(nsIScrollableView* aScrollable) = 0;
 	NS_IMETHOD ScrollPositionDidChange(nsIScrollableView* aScrollable, nscoord aX, nscoord aY) = 0;
 };
 

view/src/nsScrollPortView.cpp

@@ -647,6 +647,18 @@ NS_IMETHODIMP nsScrollPortView::ScrollToImpl(nscoord aX, nscoord aY, PRUint32 aU
   // so don't update their positions
   scrolledView->SetPositionIgnoringChildWidgets(-aX, -aY);
       
+  // notify the listeners.
+  if (nsnull != mListeners) {
+    if (NS_SUCCEEDED(mListeners->Count(&listenerCount))) {
+      for (PRUint32 i = 0; i < listenerCount; i++) {
+        if (NS_SUCCEEDED(mListeners->QueryElementAt(i, kScrollPositionListenerIID, (void**)&listener))) {
+          listener->ViewPositionDidChange(this);
+          NS_RELEASE(listener);
+        }
+      }
+    }
+  }
+
   nsPoint twipsDelta(aX - mOffsetX, aY - mOffsetY);
 
   // store the new position