Move existing PortalBrowserTest.* to use PortalActivatedObserver.

The separate observer has a couple of advantages.

Firstly, it allows waiting and access to occur after the
content::Portal (and its interceptor) may have been destroyed.
Instead, it's valid as long as the Portal is valid when the
observer is created.

Secondly, it allows examination of the activate result.

Since there are existing tests which rely on the ability to
observe activation immediately after a portal is created or
adopted (ideally we would more explicitly sequence these events
in the test, but this is not always possible), the created
observer is augmented with the ability to run a callback
immediately (rather than after the run loop quits, which is
subtle due to other work in the same task and nested run loops).
This is used to create a PortalActivatedObserver immediately
when the Portal is created, before the browser UI thread can
process any task which might either activate or destroy the Portal.

This mostly makes PortalInterceptorForTesting an implementation
detail of the more specific observers.

Change-Id: I6ee3bd8e847235654cc4a063659818e2e970c2b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900258
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Lucas Gadani <lfg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716374}

content/test/portal/portal_created_observer.cc

@@ -45,8 +45,7 @@ void PortalCreatedObserver::CreatePortal(
   std::move(callback).Run(proxy_host->GetRoutingID(), portal_->portal_token(),
                           portal_->GetDevToolsFrameToken());
 
-  if (run_loop_)
-    run_loop_->Quit();
+  DidCreatePortal();
 }
 
 void PortalCreatedObserver::AdoptPortal(
@@ -62,8 +61,7 @@ void PortalCreatedObserver::AdoptPortal(
       proxy_host->frame_tree_node()->current_replication_state(),
       portal->GetDevToolsFrameToken());
 
-  if (run_loop_)
-    run_loop_->Quit();
+  DidCreatePortal();
 }
 
 Portal* PortalCreatedObserver::WaitUntilPortalCreated() {
@@ -83,4 +81,12 @@ Portal* PortalCreatedObserver::WaitUntilPortalCreated() {
   return portal;
 }
 
+void PortalCreatedObserver::DidCreatePortal() {
+  DCHECK(portal_);
+  if (!created_cb_.is_null())
+    std::move(created_cb_).Run(portal_);
+  if (run_loop_)
+    run_loop_->Quit();
+}
+
 }  // namespace content

content/test/portal/portal_created_observer.h

@@ -5,6 +5,7 @@
 #ifndef CONTENT_TEST_PORTAL_PORTAL_CREATED_OBSERVER_H_
 #define CONTENT_TEST_PORTAL_PORTAL_CREATED_OBSERVER_H_
 
+#include "base/callback.h"
 #include "content/common/frame.mojom-test-utils.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
 #include "mojo/public/cpp/bindings/pending_associated_remote.h"
@@ -28,6 +29,15 @@ class PortalCreatedObserver : public mojom::FrameHostInterceptorForTesting {
   explicit PortalCreatedObserver(RenderFrameHostImpl* render_frame_host_impl);
   ~PortalCreatedObserver() override;
 
+  // If set, callback will be run immediately when the portal is created, before
+  // any subsequent tasks which may run before the run loop quits -- or even
+  // before it starts, if multiple events are being waited for one after
+  // another.
+  void set_created_callback(base::OnceCallback<void(Portal*)> created_cb) {
+    DCHECK(!portal_) << "Too late to register a created callback.";
+    created_cb_ = std::move(created_cb);
+  }
+
   // mojom::FrameHostInterceptorForTesting
   mojom::FrameHost* GetForwardingInterface() override;
   void CreatePortal(
@@ -37,11 +47,15 @@ class PortalCreatedObserver : public mojom::FrameHostInterceptorForTesting {
   void AdoptPortal(const base::UnguessableToken& portal_token,
                    AdoptPortalCallback callback) override;
 
+  // Wait until a portal is created (either newly or through adoption).
   Portal* WaitUntilPortalCreated();
 
  private:
+  void DidCreatePortal();
+
   RenderFrameHostImpl* render_frame_host_impl_;
   mojom::FrameHost* old_impl_;
+  base::OnceCallback<void(Portal*)> created_cb_;
   base::RunLoop* run_loop_ = nullptr;
   Portal* portal_ = nullptr;
 };

content/test/portal/portal_interceptor_for_testing.cc

@@ -80,15 +80,9 @@ blink::mojom::Portal* PortalInterceptorForTesting::GetForwardingInterface() {
 
 void PortalInterceptorForTesting::Activate(blink::TransferableMessage data,
                                            ActivateCallback callback) {
-  portal_activated_ = true;
   for (Observer& observer : observers_->data)
     observer.OnPortalActivate();
 
-  if (run_loop_) {
-    run_loop_->Quit();
-    run_loop_ = nullptr;
-  }
-
   // |this| can be destroyed after Activate() is called.
   portal_->Activate(
       std::move(data),
@@ -116,13 +110,4 @@ void PortalInterceptorForTesting::Navigate(
   portal_->Navigate(url, std::move(referrer), std::move(callback));
 }
 
-void PortalInterceptorForTesting::WaitForActivate() {
-  if (portal_activated_)
-    return;
-
-  base::RunLoop run_loop;
-  run_loop_ = &run_loop;
-  run_loop.Run();
-}
-
 }  // namespace content

content/test/portal/portal_interceptor_for_testing.h

@@ -19,10 +19,6 @@
 #include "third_party/blink/public/mojom/portal/portal.mojom-test-utils.h"
 #include "third_party/blink/public/mojom/portal/portal.mojom.h"
 
-namespace base {
-class RunLoop;
-}  // namespace base
-
 namespace content {
 
 class RenderFrameHostImpl;
@@ -66,10 +62,6 @@ class PortalInterceptorForTesting final
     navigate_callback_ = std::move(callback);
   }
 
-  // TODO(jbroman): Migrate callers to the more flexible
-  // PortalActivatedObserver.
-  void WaitForActivate();
-
   // Test getters.
   content::Portal* GetPortal() { return portal_.get(); }
   WebContentsImpl* GetPortalContents() { return portal_->GetPortalContents(); }
@@ -95,8 +87,6 @@ class PortalInterceptorForTesting final
       observers_;
   std::unique_ptr<content::Portal> portal_;
   NavigateCallback navigate_callback_;
-  bool portal_activated_ = false;
-  base::RunLoop* run_loop_ = nullptr;
   base::WeakPtrFactory<PortalInterceptorForTesting> weak_ptr_factory_{this};
 };