PM: Fix a stale comment in PageNode and add testing.

This is a followup to http://crrev.com/c/2231265, where I missed Chris'
comment on this staleness in the PageNode declaration.

Change-Id: I9e678a3f9f27c5b9afc5357985b59f8d947a9b4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2236012
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776092}

components/performance_manager/graph/frame_node_impl_unittest.cc

@@ -467,7 +467,10 @@ class LenientMockPageObserver : public PageNode::ObserverDefaultImpl {
   LenientMockPageObserver() = default;
   ~LenientMockPageObserver() override = default;
 
+  MOCK_METHOD1(OnBeforePageNodeRemoved, void(const PageNode* page_node));
+
   // Note that opener functionality is actually tested in the
+  // performance_manager_browsertest.
   MOCK_METHOD3(OnOpenerFrameNodeChanged,
                void(const PageNode*, const FrameNode*, OpenedType));
 };
@@ -623,6 +626,30 @@ TEST_F(FrameNodeImplTest, OpenerRelationships) {
   EXPECT_TRUE(frameA2->opened_page_nodes().empty());
   testing::Mock::VerifyAndClear(&obs);
 
+  // verify that the opener relationship is torn down before any node removal
+  // notification arrives.
+  EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), nullptr,
+                                            OpenedType::kInvalid));
+  pageB->SetOpenerFrameNodeAndOpenedType(frameA2.get(), OpenedType::kPopup);
+  EXPECT_EQ(frameA2.get(), pageB->opener_frame_node());
+  EXPECT_EQ(OpenedType::kPopup, pageB->opened_type());
+  EXPECT_TRUE(frameA1->opened_page_nodes().empty());
+  EXPECT_EQ(1u, frameA2->opened_page_nodes().size());
+  EXPECT_EQ(1u, frameA2->opened_page_nodes().count(pageB.get()));
+  testing::Mock::VerifyAndClear(&obs);
+
+  {
+    ::testing::InSequence seq;
+
+    // These must occur in sequence.
+    EXPECT_CALL(obs, OnOpenerFrameNodeChanged(pageB.get(), frameA2.get(),
+                                              OpenedType::kPopup));
+    EXPECT_CALL(obs, OnBeforePageNodeRemoved(pageB.get()));
+  }
+  frameB1.reset();
+  pageB.reset();
+  testing::Mock::VerifyAndClear(&obs);
+
   graph()->RemovePageNodeObserver(&obs);
 }
 

components/performance_manager/public/graph/page_node.h

@@ -175,8 +175,7 @@ class PageNodeObserver {
   // Invoked when this page has been assigned an opener, had the opener change,
   // or had the opener removed. This can happen if a page is opened via
   // window.open, webviews, portals, etc, or when that relationship is
-  // subsequently severed or reparented. Note that this can be invoked *after*
-  // OnBeforePageNodeRemoved() if a page disappears while still attached.
+  // subsequently severed or reparented.
   virtual void OnOpenerFrameNodeChanged(const PageNode* page_node,
                                         const FrameNode* previous_opener,
                                         OpenedType previous_opened_type) = 0;