Bug 1348665 part 2 - Remove the ViewProperty and store the nsView* in a field on the relevant frame classes instead. r=tnikkel draft
authorMats Palmgren <mats@mozilla.com>
Tue, 21 Mar 2017 00:01:42 +0100
changeset 501758 61d3e2268df4db2e81193aeab81c679afeb189ce
parent 501714 d0228816ca706e48b8426593b5455bc9582d03a2
child 501759 dae805bbae6f261f55e82f98d9dc760f2207aa8d
push id50115
push usermpalmgren@mozilla.com
push dateMon, 20 Mar 2017 23:15:02 +0000
reviewerstnikkel
bugs1348665
milestone55.0a1
Bug 1348665 part 2 - Remove the ViewProperty and store the nsView* in a field on the relevant frame classes instead. r=tnikkel The relevant frame classes are: SubDocumentFrame ListControlFrame (only when used for (non-e10s?) comboboxes) PluginFrame ViewportFrame MenuPopupFrame The view is now created in the frame's Init() method, except for ViewportFrame which has its view assigned by the frame constructor via a SetView() call. MozReview-Commit-ID: 4O7Hm1yqwIp
layout/base/nsCSSFrameConstructor.cpp
layout/forms/nsListControlFrame.cpp
layout/forms/nsListControlFrame.h
layout/generic/ViewportFrame.cpp
layout/generic/ViewportFrame.h
layout/generic/nsFrame.cpp
layout/generic/nsFrame.h
layout/generic/nsIFrame.h
layout/generic/nsPluginFrame.cpp
layout/generic/nsPluginFrame.h
layout/generic/nsSubDocumentFrame.cpp
layout/generic/nsSubDocumentFrame.h
layout/xul/nsMenuPopupFrame.cpp
layout/xul/nsMenuPopupFrame.h
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3236,20 +3236,16 @@ nsCSSFrameConstructor::InitializeSelectF
 
   scrollFrame->Init(aContent, geometricParent, nullptr);
 
   if (!aBuildCombobox) {
     aState.AddChild(scrollFrame, aFrameItems, aContent,
                     aStyleContext, aParentFrame);
   }
 
-  if (aBuildCombobox) {
-    nsFrame::CreateViewForFrame(scrollFrame, true);
-  }
-
   BuildScrollFrame(aState, aContent, aStyleContext, scrolledFrame,
                    geometricParent, scrollFrame);
 
   if (aState.mFrameState) {
     // Restore frame state for the scroll frame
     RestoreFrameStateFor(scrollFrame, aState.mFrameState);
   }
 
@@ -3925,18 +3921,16 @@ nsCSSFrameConstructor::ConstructFrameFro
     if ((bits & FCDATA_MAY_NEED_SCROLLFRAME) &&
         display->IsScrollableOverflow()) {
       nsContainerFrame* scrollframe = nullptr;
       BuildScrollFrame(aState, content, styleContext, newFrame,
                        geometricParent, scrollframe);
       frameToAddToList = scrollframe;
     } else {
       InitAndRestoreFrame(aState, content, geometricParent, newFrame);
-      // See whether we need to create a view
-      nsFrame::CreateViewForFrame(newFrame, false);
       frameToAddToList = newFrame;
     }
 
     // Use frameToAddToList as the primary frame.  In the non-scrollframe case
     // they're equal, but in the scrollframe case newFrame is the scrolled
     // frame, while frameToAddToList is the scrollframe (and should be the
     // primary frame).
     primaryFrame = frameToAddToList;
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -88,16 +88,17 @@ NS_NewListControlFrame(nsIPresShell* aPr
   return it;
 }
 
 NS_IMPL_FRAMEARENA_HELPERS(nsListControlFrame)
 
 //---------------------------------------------------------
 nsListControlFrame::nsListControlFrame(nsStyleContext* aContext)
   : nsHTMLScrollFrame(aContext, false),
+    mView(nullptr),
     mMightNeedSecondPass(false),
     mHasPendingInterruptAtStartOfReflow(false),
     mDropdownCanGrow(false),
     mForceSelection(false),
     mLastDropdownComputedBSize(NS_UNCONSTRAINEDSIZE)
 {
   mComboboxFrame      = nullptr;
   mChangesSinceDragStart = false;
@@ -970,16 +971,21 @@ nsListControlFrame::SetInitialChildList(
 //---------------------------------------------------------
 void
 nsListControlFrame::Init(nsIContent*       aContent,
                          nsContainerFrame* aParent,
                          nsIFrame*         aPrevInFlow)
 {
   nsHTMLScrollFrame::Init(aContent, aParent, aPrevInFlow);
 
+  if (IsInDropDownMode()) {
+    AddStateBits(NS_FRAME_IN_POPUP);
+    CreateView();
+  }
+
   // we shouldn't have to unregister this listener because when
   // our frame goes away all these content node go away as well
   // because our frame is the only one who references them.
   // we need to hook up our listeners before the editor is initialized
   mEventListener = new nsListEventListener(this);
 
   mContent->AddSystemEventListener(NS_LITERAL_STRING("keydown"),
                                    mEventListener, false, false);
@@ -991,20 +997,16 @@ nsListControlFrame::Init(nsIContent*    
                                    mEventListener, false, false);
   mContent->AddSystemEventListener(NS_LITERAL_STRING("mousemove"),
                                    mEventListener, false, false);
 
   mStartSelectionIndex = kNothingSelected;
   mEndSelectionIndex = kNothingSelected;
 
   mLastDropdownBackstopColor = PresContext()->DefaultBackgroundColor();
-
-  if (IsInDropDownMode()) {
-    AddStateBits(NS_FRAME_IN_POPUP);
-  }
 }
 
 dom::HTMLOptionsCollection*
 nsListControlFrame::GetOptions() const
 {
   dom::HTMLSelectElement* select =
     dom::HTMLSelectElement::FromContentOrNull(mContent);
   NS_ENSURE_TRUE(select, nullptr);
--- a/layout/forms/nsListControlFrame.h
+++ b/layout/forms/nsListControlFrame.h
@@ -235,21 +235,16 @@ public:
 
   /**
    * Return true if the drop-down list can display more rows.
    * (always false if not in drop-down mode)
    */
   bool GetDropdownCanGrow() const { return mDropdownCanGrow; }
 
   /**
-   * Dropdowns need views
-   */
-  virtual bool NeedsView() override { return IsInDropDownMode(); }
-
-  /**
    * Frees statics owned by this class.
    */
   static void Shutdown();
 
 #ifdef ACCESSIBILITY
   /**
    * Post a custom DOM event for the change, so that accessibility can
    * fire a native focus event for accessibility
@@ -393,26 +388,33 @@ protected:
     return GetOptionsContainer()->BSizeOfARow();
   }
 
   /**
    * @return how many displayable options/optgroups this frame has.
    */
   uint32_t GetNumberOfRows();
 
+  nsView* GetViewInternal() const override { return mView; }
+  void SetViewInternal(nsView* aView) override { mView = aView; }
+
   // Data Members
   int32_t      mStartSelectionIndex;
   int32_t      mEndSelectionIndex;
 
-  nsIComboboxControlFrame *mComboboxFrame;
-  uint32_t     mNumDisplayRows;
+  nsIComboboxControlFrame* mComboboxFrame;
+
+  // The view is only created (& non-null) if IsInDropDownMode() is true.
+  nsView* mView;
+
+  uint32_t mNumDisplayRows;
   bool mChangesSinceDragStart:1;
   bool mButtonDown:1;
-  // Has the user selected a visible item since we showed the
-  // dropdown?
+
+  // Has the user selected a visible item since we showed the dropdown?
   bool mItemSelectionStarted:1;
 
   bool mIsAllContentHere:1;
   bool mIsAllFramesHere:1;
   bool mHasBeenInitialized:1;
   bool mNeedToReset:1;
   bool mPostChildrenLoadedReset:1;
 
--- a/layout/generic/ViewportFrame.cpp
+++ b/layout/generic/ViewportFrame.cpp
@@ -34,16 +34,18 @@ NS_QUERYFRAME_HEAD(ViewportFrame)
 NS_QUERYFRAME_TAIL_INHERITING(nsContainerFrame)
 
 void
 ViewportFrame::Init(nsIContent*       aContent,
                     nsContainerFrame* aParent,
                     nsIFrame*         aPrevInFlow)
 {
   nsContainerFrame::Init(aContent, aParent, aPrevInFlow);
+  // No need to call CreateView() here - the frame ctor will call SetView()
+  // with the ViewManager's root view, so we'll assign it in SetViewInternal().
 
   nsIFrame* parent = nsLayoutUtils::GetCrossDocParentFrame(this);
   if (parent) {
     nsFrameState state = parent->GetStateBits();
 
     mState |= state & (NS_FRAME_IN_POPUP);
   }
 }
--- a/layout/generic/ViewportFrame.h
+++ b/layout/generic/ViewportFrame.h
@@ -26,16 +26,17 @@ namespace mozilla {
 class ViewportFrame : public nsContainerFrame {
 public:
   NS_DECL_QUERYFRAME_TARGET(ViewportFrame)
   NS_DECL_QUERYFRAME
   NS_DECL_FRAMEARENA_HELPERS
 
   explicit ViewportFrame(nsStyleContext* aContext)
     : nsContainerFrame(aContext)
+    , mView(nullptr)
   {}
   virtual ~ViewportFrame() { } // useful for debugging
 
   virtual void Init(nsIContent*       aContent,
                     nsContainerFrame* aParent,
                     nsIFrame*         aPrevInFlow) override;
 
 #ifdef DEBUG
@@ -78,25 +79,30 @@ public:
    * @return the rect to use as containing block rect
    */
   nsRect AdjustReflowInputAsContainingBlock(ReflowInput* aReflowInput) const;
 
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override;
 #endif
 
-private:
-  virtual mozilla::layout::FrameChildListID GetAbsoluteListID() const override { return kFixedList; }
-
 protected:
   /**
    * Calculate how much room is available for fixed frames. That means
    * determining if the viewport is scrollable and whether the vertical and/or
    * horizontal scrollbars are visible.  Adjust the computed width/height and
    * available width for aReflowInput accordingly.
    * @return the current scroll position, or 0,0 if not scrollable
    */
   nsPoint AdjustReflowInputForScrollbars(ReflowInput* aReflowInput) const;
+
+  nsView* GetViewInternal() const override { return mView; }
+  void SetViewInternal(nsView* aView) override { mView = aView; }
+
+private:
+  virtual mozilla::layout::FrameChildListID GetAbsoluteListID() const override { return kFixedList; }
+
+  nsView* mView;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_ViewportFrame_h
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -685,22 +685,18 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
   if (StyleDisplay()->mPosition == NS_STYLE_POSITION_STICKY) {
     StickyScrollContainer* ssc =
       StickyScrollContainer::GetStickyScrollContainerForFrame(this);
     if (ssc) {
       ssc->RemoveFrame(this);
     }
   }
 
-  // Get the view pointer now before the frame properties disappear
-  // when we call NotifyDestroyingFrame()
-  nsView* view = GetView();
   nsPresContext* presContext = PresContext();
-
-  nsIPresShell *shell = presContext->GetPresShell();
+  nsIPresShell* shell = presContext->GetPresShell();
   if (mState & NS_FRAME_OUT_OF_FLOW) {
     nsPlaceholderFrame* placeholder =
       shell->FrameManager()->GetPlaceholderFrameFor(this);
     NS_ASSERTION(!placeholder || (aDestructRoot != this),
                  "Don't call Destroy() on OOFs, call Destroy() on the placeholder.");
     NS_ASSERTION(!placeholder ||
                  nsLayoutUtils::IsProperAncestorFrame(aDestructRoot, placeholder),
                  "Placeholder relationship should have been torn down already; "
@@ -780,21 +776,19 @@ nsFrame::DestroyFrom(nsIFrame* aDestruct
   PresContext()->GetPresShell()->RemoveFrameFromApproximatelyVisibleList(this);
 
   shell->NotifyDestroyingFrame(this);
 
   if (mState & NS_FRAME_EXTERNAL_REFERENCE) {
     shell->ClearFrameRefs(this);
   }
 
+  nsView* view = GetView();
   if (view) {
-    // Break association between view and frame
     view->SetFrame(nullptr);
-
-    // Destroy the view
     view->Destroy();
   }
 
   // Make sure that our deleted frame can't be returned from GetPrimaryFrame()
   if (isPrimaryFrame) {
     mContent->SetPrimaryFrame(nullptr);
   }
 
@@ -1067,62 +1061,53 @@ nsIFrame::SyncFrameViewProperties(nsPres
   } else {
     autoZIndex = true;
   }
 
   vm->SetViewZIndex(aView, autoZIndex, zIndex);
 }
 
 void
-nsFrame::CreateViewForFrame(nsIFrame* aFrame,
-                            bool aForce)
-{
-  if (aFrame->HasView()) {
-    return;
-  }
-
-  // If we don't yet have a view, see if we need a view
-  if (!aForce && !aFrame->NeedsView()) {
-    // don't need a view
-    return;
-  }
-
-  nsView* parentView = aFrame->GetParent()->GetClosestView();
+nsFrame::CreateView()
+{
+  MOZ_ASSERT(!HasView());
+
+  nsView* parentView = GetParent()->GetClosestView();
   NS_ASSERTION(parentView, "no parent with view");
 
   nsViewManager* viewManager = parentView->GetViewManager();
   NS_ASSERTION(viewManager, "null view manager");
 
   // Create a view
-  nsView* view = viewManager->CreateView(aFrame->GetRect(), parentView);
-
-  SyncFrameViewProperties(aFrame->PresContext(), aFrame, nullptr, view);
-
-  nsView* insertBefore = nsLayoutUtils::FindSiblingViewFor(parentView, aFrame);
+  nsView* view = viewManager->CreateView(GetRect(), parentView);
+
+  SyncFrameViewProperties(PresContext(), this, nullptr, view);
+
+  nsView* insertBefore = nsLayoutUtils::FindSiblingViewFor(parentView, this);
   // we insert this view 'above' the insertBefore view, unless insertBefore is null,
   // in which case we want to call with aAbove == false to insert at the beginning
   // in document order
   viewManager->InsertChild(parentView, view, insertBefore, insertBefore != nullptr);
 
   // REVIEW: Don't create a widget for fixed-pos elements anymore.
   // ComputeRepaintRegionForCopy will calculate the right area to repaint
   // when we scroll.
   // Reparent views on any child frames (or their descendants) to this
   // view. We can just call ReparentFrameViewTo on this frame because
   // we know this frame has no view, so it will crawl the children. Also,
   // we know that any descendants with views must have 'parentView' as their
   // parent view.
-  ReparentFrameViewTo(aFrame, viewManager, view, parentView);
+  ReparentFrameViewTo(this, viewManager, view, parentView);
 
   // Remember our view
-  aFrame->SetView(view);
+  SetView(view);
 
   NS_FRAME_LOG(NS_FRAME_TRACE_CALLS,
-               ("nsContainerFrame::CreateViewForFrame: frame=%p view=%p",
-                aFrame, view));
+               ("nsFrame::CreateView: frame=%p view=%p",
+                this, view));
 }
 
 // MSVC fails with link error "one or more multiply defined symbols found",
 // gcc fails with "hidden symbol `nsIFrame::kPrincipalList' isn't defined"
 // etc if they are not defined.
 #ifndef _MSC_VER
 // static nsIFrame constants; initialized in the header file.
 const nsIFrame::ChildListID nsIFrame::kPrincipalList;
@@ -5979,63 +5964,49 @@ nsIFrame* nsIFrame::GetTailContinuation(
        next && !(next->GetStateBits() & NS_FRAME_IS_OVERFLOW_CONTAINER);
        next = frame->GetNextContinuation())  {
     frame = next;
   }
   NS_POSTCONDITION(frame, "illegal state in continuation chain.");
   return frame;
 }
 
-NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(ViewProperty, nsView)
-
 // Associated view object
-nsView*
-nsIFrame::GetView() const
-{
-  // Check the frame state bit and see if the frame has a view
-  if (!(GetStateBits() & NS_FRAME_HAS_VIEW))
-    return nullptr;
-
-  // Check for a property on the frame
-  nsView* value = Properties().Get(ViewProperty());
-  NS_ASSERTION(value, "frame state bit was set but frame has no view");
-  return value;
-}
-
-nsresult
+void
 nsIFrame::SetView(nsView* aView)
 {
   if (aView) {
     aView->SetFrame(this);
 
 #ifdef DEBUG
     nsIAtom* frameType = GetType();
-    NS_ASSERTION(frameType == nsGkAtoms::scrollFrame ||
-                 frameType == nsGkAtoms::subDocumentFrame ||
+    NS_ASSERTION(frameType == nsGkAtoms::subDocumentFrame ||
                  frameType == nsGkAtoms::listControlFrame ||
                  frameType == nsGkAtoms::objectFrame ||
                  frameType == nsGkAtoms::viewportFrame ||
                  frameType == nsGkAtoms::menuPopupFrame,
                  "Only specific frame types can have an nsView");
 #endif
 
-    // Set a property on the frame
-    Properties().Set(ViewProperty(), aView);
+    // Store the view on the frame.
+    SetViewInternal(aView);
 
     // Set the frame state bit that says the frame has a view
     AddStateBits(NS_FRAME_HAS_VIEW);
 
     // Let all of the ancestors know they have a descendant with a view.
     for (nsIFrame* f = GetParent();
          f && !(f->GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW);
          f = f->GetParent())
       f->AddStateBits(NS_FRAME_HAS_CHILD_WITH_VIEW);
-  }
-
-  return NS_OK;
+  } else {
+    MOZ_ASSERT_UNREACHABLE("Destroying a view while the frame is alive?");
+    RemoveStateBits(NS_FRAME_HAS_VIEW);
+    SetViewInternal(nullptr);
+  }
 }
 
 // Find the first geometric parent that has a view
 nsIFrame* nsIFrame::GetAncestorWithView() const
 {
   for (nsIFrame* f = GetParent(); nullptr != f; f = f->GetParent()) {
     if (f->HasView()) {
       return f;
--- a/layout/generic/nsFrame.h
+++ b/layout/generic/nsFrame.h
@@ -591,21 +591,20 @@ protected:
 
   int16_t DisplaySelection(nsPresContext* aPresContext, bool isOkToTurnOn = false);
   
   // Style post processing hook
   void DidSetStyleContext(nsStyleContext* aOldStyleContext) override;
 
 public:
   /**
-   * Helper method to wrap views around frames. Used by containers
-   * under special circumstances (can be used by leaf frames as well)
+   * Helper method to create a view for a frame.  Only used by a few sub-classes
+   * that need a view.
    */
-  static void CreateViewForFrame(nsIFrame* aFrame,
-                                 bool aForce);
+  void CreateView();
 
   //given a frame five me the first/last leaf available
   //XXX Robert O'Callahan wants to move these elsewhere
   static void GetLastLeaf(nsPresContext* aPresContext, nsIFrame **aFrame);
   static void GetFirstLeaf(nsPresContext* aPresContext, nsIFrame **aFrame);
 
   // Return the line number of the aFrame, and (optionally) the containing block
   // frame.
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -608,18 +608,18 @@ public:
 
   /**
    * Called to initialize the frame. This is called immediately after creating
    * the frame.
    *
    * If the frame is a continuing frame, then aPrevInFlow indicates the previous
    * frame (the frame that was split).
    *
-   * If you want a view associated with your frame, you should create the view
-   * after Init() has returned.
+   * Each subclass that need a view should override this method and call
+   * CreateView() after calling its base class Init().
    *
    * @param   aContent the content object associated with the frame
    * @param   aParent the parent frame
    * @param   aPrevInFlow the prev-in-flow frame
    */
   virtual void Init(nsIContent*       aContent,
                     nsContainerFrame* aParent,
                     nsIFrame*         aPrevInFlow) = 0;
@@ -1602,21 +1602,16 @@ public:
    *    DISPLAY_CHILD_FORCE_STACKING_CONTEXT and DISPLAY_CHILD_INLINE
    */
   void BuildDisplayListForChild(nsDisplayListBuilder*   aBuilder,
                                 nsIFrame*               aChild,
                                 const nsRect&           aDirtyRect,
                                 const nsDisplayListSet& aLists,
                                 uint32_t                aFlags = 0);
 
-  /**
-   * Does this frame need a view?
-   */
-  virtual bool NeedsView() { return false; }
-
   bool RefusedAsyncAnimation() const
   {
     return Properties().Get(RefusedAsyncAnimationProperty());
   }
 
   /**
    * Returns true if this frame is transformed (e.g. has CSS or SVG transforms)
    * or if its parent is an SVG frame that has children-only transforms (e.g.
@@ -2440,24 +2435,41 @@ public:
   /**
    * Returns true if the frame contains any non-collapsed characters.
    * This method is only available for text frames, and it will return false
    * for all other frame types.
    */
   virtual bool HasAnyNoncollapsedCharacters()
   { return false; }
 
-  /**
-   * Accessor functions to get/set the associated view object
-   *
-   * GetView returns non-null if and only if |HasView| returns true.
-   */
+  //
+  // Accessor functions to an associated view object:
+  //
   bool HasView() const { return !!(mState & NS_FRAME_HAS_VIEW); }
-  nsView* GetView() const;
-  nsresult SetView(nsView* aView);
+protected:
+  virtual nsView* GetViewInternal() const
+  {
+    MOZ_ASSERT_UNREACHABLE("method should have been overridden by subclass");
+    return nullptr;
+  }
+  virtual void SetViewInternal(nsView* aView)
+  {
+    MOZ_ASSERT_UNREACHABLE("method should have been overridden by subclass");
+  }
+public:
+  nsView* GetView() const
+  {
+    if (MOZ_LIKELY(!HasView())) {
+      return nullptr;
+    }
+    nsView* view = GetViewInternal();
+    MOZ_ASSERT(view, "GetViewInternal() should agree with HasView()");
+    return view;
+  }
+  void SetView(nsView* aView);
 
   /**
    * Find the closest view (on |this| or an ancestor).
    * If aOffset is non-null, it will be set to the offset of |this|
    * from the returned view.
    */
   nsView* GetClosestView(nsPoint* aOffset = nullptr) const;
 
--- a/layout/generic/nsPluginFrame.cpp
+++ b/layout/generic/nsPluginFrame.cpp
@@ -148,16 +148,17 @@ protected:
 };
 
 nsPluginFrame::nsPluginFrame(nsStyleContext* aContext)
   : nsFrame(aContext)
   , mInstanceOwner(nullptr)
   , mInnerView(nullptr)
   , mBackgroundSink(nullptr)
   , mReflowCallbackPosted(false)
+  , mView(nullptr)
 {
   MOZ_LOG(sPluginFrameLog, LogLevel::Debug,
          ("Created new nsPluginFrame %p\n", this));
 }
 
 nsPluginFrame::~nsPluginFrame()
 {
   MOZ_LOG(sPluginFrameLog, LogLevel::Debug,
@@ -189,16 +190,17 @@ void
 nsPluginFrame::Init(nsIContent*       aContent,
                     nsContainerFrame* aParent,
                     nsIFrame*         aPrevInFlow)
 {
   MOZ_LOG(sPluginFrameLog, LogLevel::Debug,
          ("Initializing nsPluginFrame %p for content %p\n", this, aContent));
 
   nsFrame::Init(aContent, aParent, aPrevInFlow);
+  CreateView();
 }
 
 void
 nsPluginFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
   if (mReflowCallbackPosted) {
     PresContext()->PresShell()->CancelReflowCallback(this);
   }
--- a/layout/generic/nsPluginFrame.h
+++ b/layout/generic/nsPluginFrame.h
@@ -91,18 +91,16 @@ public:
   virtual nsIAtom* GetType() const override;
 
   virtual bool IsFrameOfType(uint32_t aFlags) const override
   {
     return nsFrame::IsFrameOfType(aFlags &
       ~(nsIFrame::eReplaced | nsIFrame::eReplacedSizing));
   }
 
-  virtual bool NeedsView() override { return true; }
-
 #ifdef DEBUG_FRAME_DUMP
   virtual nsresult GetFrameName(nsAString& aResult) const override;
 #endif
 
   virtual void DestroyFrom(nsIFrame* aDestructRoot) override;
 
   virtual void DidSetStyleContext(nsStyleContext* aOldStyleContext) override;
 
@@ -266,16 +264,19 @@ protected:
                    const nsRect& aDirtyRect, const nsRect& aPluginRect);
 
   void NotifyPluginReflowObservers();
 
   friend class nsPluginInstanceOwner;
   friend class nsDisplayPlugin;
   friend class PluginBackgroundSink;
 
+  nsView* GetViewInternal() const override { return mView; }
+  void SetViewInternal(nsView* aView) override { mView = aView; }
+
 private:
   // Registers the plugin for a geometry update, and requests a geometry
   // update. This caches the root pres context in
   // mRootPresContextRegisteredWith, so that we can be sure we unregister
   // from the right root prest context in UnregisterPluginForGeometryUpdates.
   void RegisterPluginForGeometryUpdates();
 
   // Unregisters the plugin for geometry updated with the root pres context
@@ -328,16 +329,18 @@ private:
 
   // We keep this reference to ensure we can always unregister the
   // plugins we register on the root PresContext.
   // This is only non-null while we have a plugin registered for geometry
   // updates.
   RefPtr<nsRootPresContext> mRootPresContextRegisteredWith;
 
   mozilla::UniquePtr<PluginFrameDidCompositeObserver> mDidCompositeObserver;
+
+  nsView* mView;
 };
 
 class nsDisplayPlugin : public nsDisplayItem {
 public:
   nsDisplayPlugin(nsDisplayListBuilder* aBuilder, nsIFrame* aFrame)
     : nsDisplayItem(aBuilder, aFrame)
   {
     MOZ_COUNT_CTOR(nsDisplayPlugin);
--- a/layout/generic/nsSubDocumentFrame.cpp
+++ b/layout/generic/nsSubDocumentFrame.cpp
@@ -55,16 +55,17 @@ GetDocumentFromView(nsView* aView)
 
   nsViewManager* vm = aView->GetViewManager();
   nsIPresShell* ps =  vm ? vm->GetPresShell() : nullptr;
   return ps ? ps->GetDocument() : nullptr;
 }
 
 nsSubDocumentFrame::nsSubDocumentFrame(nsStyleContext* aContext)
   : nsAtomicContainerFrame(aContext)
+  , mOuterView(nullptr)
   , mInnerView(nullptr)
   , mIsInline(false)
   , mPostedReflowCallback(false)
   , mDidCreateDoc(false)
   , mCallingShow(false)
 {
 }
 
@@ -116,27 +117,20 @@ nsSubDocumentFrame::Init(nsIContent*    
     // If layout.show_previous_page is true then during loading of a new page we
     // will draw the previous page if the new page has painting suppressed.
     Preferences::AddBoolVarCache(&sShowPreviousPage, "layout.show_previous_page", true);
     addedShowPreviousPage = true;
   }
 
   nsAtomicContainerFrame::Init(aContent, aParent, aPrevInFlow);
 
-  // We are going to create an inner view.  If we need a view for the
-  // OuterFrame but we wait for the normal view creation path in
-  // nsCSSFrameConstructor, then we will lose because the inner view's
-  // parent will already have been set to some outer view (e.g., the
-  // canvas) when it really needs to have this frame's view as its
-  // parent. So, create this frame's view right away, whether we
-  // really need it or not, and the inner view will get it as the
-  // parent.
-  if (!HasView()) {
-    nsFrame::CreateViewForFrame(this, true);
-  }
+  // CreateView() creates this frame's view, stored in mOuterView.  It needs to
+  // be created first since it's the parent of the inner view, stored in
+  // mInnerView.
+  CreateView();
   EnsureInnerView();
 
   // Set the primary frame now so that nsDocumentViewer::FindContainerView
   // called from within EndSwapDocShellsForViews below can find it if needed.
   aContent->SetPrimaryFrame(this);
 
   // If we have a detached subdoc's root view on our frame loader, re-insert
   // it into the view tree. This happens when we've been reframed, and
--- a/layout/generic/nsSubDocumentFrame.h
+++ b/layout/generic/nsSubDocumentFrame.h
@@ -152,17 +152,21 @@ protected:
    * and our sub-document has an intrinsic size. The frame returned is the
    * frame for the document element of the document we're embedding.
    *
    * Called "Obtain*" and not "Get*" because of comment on GetDocShell that
    * says it should be called ObtainDocShell because of its side effects.
    */
   nsIFrame* ObtainIntrinsicSizeFrame();
 
+  nsView* GetViewInternal() const override { return mOuterView; }
+  void SetViewInternal(nsView* aView) override { mOuterView = aView; }
+
   RefPtr<nsFrameLoader> mFrameLoader;
+  nsView* mOuterView;
   nsView* mInnerView;
   bool mIsInline;
   bool mPostedReflowCallback;
   bool mDidCreateDoc;
   bool mCallingShow;
 };
 
 #endif /* NSSUBDOCUMENTFRAME_H_ */
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -89,16 +89,17 @@ NS_QUERYFRAME_HEAD(nsMenuPopupFrame)
 NS_QUERYFRAME_TAIL_INHERITING(nsBoxFrame)
 
 //
 // nsMenuPopupFrame ctor
 //
 nsMenuPopupFrame::nsMenuPopupFrame(nsStyleContext* aContext)
   :nsBoxFrame(aContext),
   mCurrentMenu(nullptr),
+  mView(nullptr),
   mPrefSize(-1, -1),
   mLastClientOffset(0, 0),
   mPopupType(ePopupTypePanel),
   mPopupState(ePopupClosed),
   mPopupAlignment(POPUPALIGNMENT_NONE),
   mPopupAnchor(POPUPALIGNMENT_NONE),
   mPosition(POPUPPOSITION_UNKNOWN),
   mConsumeRollupEvent(PopupBoxObject::ROLLUP_DEFAULT),
--- a/layout/xul/nsMenuPopupFrame.h
+++ b/layout/xul/nsMenuPopupFrame.h
@@ -528,16 +528,19 @@ protected:
       ? mAnchorContent->GetPrimaryFrame()->StyleVisibility()->mDirection == NS_STYLE_DIRECTION_RTL
       : StyleVisibility()->mDirection == NS_STYLE_DIRECTION_RTL;
   }
 
   // Create a popup view for this frame. The view is added a child of the root
   // view, and is initially hidden.
   void CreatePopupView();
 
+  nsView* GetViewInternal() const override { return mView; }
+  void SetViewInternal(nsView* aView) override { mView = aView; }
+
   // Returns true if the popup should try to remain at the same relative
   // location as the anchor while it is open. If the anchor becomes hidden
   // either directly or indirectly because a parent popup or other element
   // is no longer visible, or a parent deck page is changed, the popup hides
   // as well. The second variation also sets the anchor rectangle, relative to
   // the popup frame.
   bool ShouldFollowAnchor();
 public:
@@ -550,16 +553,17 @@ protected:
   // different document than the popup.
   nsCOMPtr<nsIContent> mAnchorContent;
 
   // the content that triggered the popup, typically the node where the mouse
   // was clicked. It will be cleared when the popup is hidden.
   nsCOMPtr<nsIContent> mTriggerContent;
 
   nsMenuFrame* mCurrentMenu; // The current menu that is active.
+  nsView* mView;
 
   RefPtr<nsXULPopupShownEvent> mPopupShownDispatcher;
 
   // The popup's screen rectangle in app units.
   nsIntRect mUsedScreenRect;
 
   // A popup's preferred size may be different than its actual size stored in
   // mRect in the case where the popup was resized because it was too large