Commit 63ecb0ca authored by Kevin McNee's avatar Kevin McNee Committed by Commit Bot

Portals: Basic linear history model

Implements a basic ability to navigate back and forward across a portal
activation based on a linear model of portal history.

Upon activation, only a portal contents' last committed entry is added
to user facing session history. When the portal contents is swapped in,
its NavigationController copies the navigation entries of the
predecessor and combines them with its last committed navigation.

Note that this does not cover the full design. Notably, the
replace/traverse options of activation are not implemented. If we want
to require navigation in a portal to be done with replacement, that's
not implemented. Various races/edge cases are not yet addressed or
cause us to bail out on the transfer of history entries for now.

Bug: 914108
Change-Id: Ie44f8c77b80eb734db519a57ab63ad88a673b862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807192
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719996}
parent ad5a9ab3
......@@ -208,11 +208,22 @@ void Portal::Navigate(const GURL& url,
// https://github.com/WICG/portals/issues/150
NavigationDownloadPolicy download_policy;
// Navigations in portals do not affect the host's session history. Upon
// activation, only the portal's last committed entry is merged with the
// host's session history. Hence, a portal maintaining multiple session
// history entries is not useful and would introduce unnecessary complexity.
// We therefore have portal navigations done with replacement, so that we only
// have one entry at a time.
// TODO(mcnee): A portal can still self-navigate without replacement. Fix this
// so that we can enforce this as an invariant.
constexpr bool should_replace_entry = true;
portal_root->navigator()->NavigateFromFrameProxy(
portal_frame, url, owner_render_frame_host_->GetLastCommittedOrigin(),
owner_render_frame_host_->GetSiteInstance(),
mojo::ConvertTo<Referrer>(referrer), ui::PAGE_TRANSITION_LINK, false,
download_policy, "GET", nullptr, "", nullptr, false);
mojo::ConvertTo<Referrer>(referrer), ui::PAGE_TRANSITION_LINK,
should_replace_entry, download_policy, "GET", nullptr, "", nullptr,
false);
std::move(callback).Run();
}
......@@ -225,6 +236,61 @@ void FlushTouchEventQueues(RenderWidgetHostImpl* host) {
while (RenderWidgetHost* child_widget = child_widgets->GetNextHost())
FlushTouchEventQueues(static_cast<RenderWidgetHostImpl*>(child_widget));
}
// Copies |predecessor_contents|'s navigation entries to
// |activated_contents|. |activated_contents| will have its last committed entry
// combined with the entries in |predecessor_contents|. |predecessor_contents|
// will only keep its last committed entry.
// TODO(914108): This currently only covers the basic cases for history
// traversal across portal activations. The design is still being discussed.
void TakeHistoryForActivation(WebContentsImpl* activated_contents,
WebContentsImpl* predecessor_contents) {
NavigationControllerImpl& activated_controller =
activated_contents->GetController();
NavigationControllerImpl& predecessor_controller =
predecessor_contents->GetController();
// Activation would have discarded any pending entry in the host contents.
DCHECK(!predecessor_controller.GetPendingEntry());
// TODO(mcnee): Don't allow activation of an empty contents (see
// https://crbug.com/942198).
if (!activated_controller.GetLastCommittedEntry()) {
DLOG(WARNING) << "An empty portal WebContents was activated.";
return;
}
// If the predecessor has no committed entries (e.g. by using window.open()
// and then activating a portal from about:blank), there's nothing to do here.
// TODO(mcnee): This should also be disallowed.
if (!predecessor_controller.GetLastCommittedEntry()) {
return;
}
// TODO(mcnee): Determine how to deal with a transient entry.
if (predecessor_controller.GetTransientEntry() ||
activated_controller.GetTransientEntry()) {
return;
}
// TODO(mcnee): Once we enforce that a portal contents does not build up its
// own history, make this DCHECK that we only have a single committed entry,
// possibly with a new pending entry.
if (activated_controller.GetPendingEntryIndex() != -1) {
return;
}
DCHECK(activated_controller.CanPruneAllButLastCommitted());
// TODO(mcnee): Allow for portal activations to replace history entries and to
// traverse existing history entries.
activated_controller.CopyStateFromAndPrune(&predecessor_controller,
false /* replace_entry */);
// The predecessor may be adopted as a portal, so it should now only have a
// single committed entry.
DCHECK(predecessor_controller.CanPruneAllButLastCommitted());
predecessor_controller.PruneAllButLastCommitted();
}
} // namespace
void Portal::Activate(blink::TransferableMessage data,
......@@ -306,6 +372,9 @@ void Portal::Activate(blink::TransferableMessage data,
FlushTouchEventQueues(outer_contents_main_frame_view->host());
}
TakeHistoryForActivation(static_cast<WebContentsImpl*>(portal_contents.get()),
outer_contents);
std::unique_ptr<WebContents> predecessor_web_contents =
delegate->SwapWebContents(outer_contents, std::move(portal_contents),
true, is_loading);
......
......@@ -1251,6 +1251,149 @@ IN_PROC_BROWSER_TEST_F(PortalBrowserTest, MisbehavingRendererActivated) {
EXPECT_EQ(bad_message::RPH_MOJO_PROCESS_ERROR, kill_waiter.Wait());
}
// Test that user facing session history is retained after a portal activation.
// Before activation, we have the host WebContents's navigation entries, which
// is the session history presented to the user, plus the portal WebContents's
// navigation entry, which is not presented as part of that session history.
// Upon activation, the host WebContents's navigation entries are merged into
// the activated portal's WebContents. The resulting session history in the
// activated WebContents is presented to the user.
IN_PROC_BROWSER_TEST_F(PortalBrowserTest, PortalHistoryWithActivation) {
// We have an additional navigation entry in the portal host's contents, so
// that we test that we're retaining more than just the last committed entry
// of the portal host.
GURL previous_url(
embedded_test_server()->GetURL("portal.test", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), previous_url));
GURL main_url(embedded_test_server()->GetURL("portal.test", "/title2.html"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
WebContentsImpl* web_contents_impl =
static_cast<WebContentsImpl*>(shell()->web_contents());
RenderFrameHostImpl* main_frame = web_contents_impl->GetMainFrame();
GURL portal_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
Portal* portal = CreatePortalToUrl(web_contents_impl, portal_url);
WebContentsImpl* portal_contents = portal->GetPortalContents();
NavigationControllerImpl& main_controller =
web_contents_impl->GetController();
NavigationControllerImpl& portal_controller =
portal_contents->GetController();
EXPECT_EQ(2, main_controller.GetEntryCount());
ASSERT_TRUE(main_controller.GetLastCommittedEntry());
EXPECT_EQ(main_url, main_controller.GetLastCommittedEntry()->GetURL());
EXPECT_EQ(1, portal_controller.GetEntryCount());
ASSERT_TRUE(portal_controller.GetLastCommittedEntry());
EXPECT_EQ(portal_url, portal_controller.GetLastCommittedEntry()->GetURL());
PortalActivatedObserver activated_observer(portal);
ASSERT_TRUE(
ExecJs(main_frame, "document.querySelector('portal').activate();"));
activated_observer.WaitForActivate();
NavigationControllerImpl& activated_controller = portal_controller;
ASSERT_EQ(3, activated_controller.GetEntryCount());
ASSERT_EQ(2, activated_controller.GetLastCommittedEntryIndex());
EXPECT_EQ(previous_url, activated_controller.GetEntryAtIndex(0)->GetURL());
EXPECT_EQ(main_url, activated_controller.GetEntryAtIndex(1)->GetURL());
EXPECT_EQ(portal_url, activated_controller.GetEntryAtIndex(2)->GetURL());
}
// Test that we may go back/forward across a portal activation as though it
// were a regular navigation.
IN_PROC_BROWSER_TEST_F(PortalBrowserTest,
PortalHistoryActivateAndGoBackForward) {
GURL main_url(embedded_test_server()->GetURL("portal.test", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
WebContentsImpl* web_contents_impl =
static_cast<WebContentsImpl*>(shell()->web_contents());
RenderFrameHostImpl* main_frame = web_contents_impl->GetMainFrame();
GURL portal_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
Portal* portal = CreatePortalToUrl(web_contents_impl, portal_url);
PortalActivatedObserver activated_observer(portal);
ASSERT_TRUE(
ExecJs(main_frame, "document.querySelector('portal').activate();"));
activated_observer.WaitForActivate();
web_contents_impl = static_cast<WebContentsImpl*>(shell()->web_contents());
NavigationControllerImpl& controller = web_contents_impl->GetController();
ASSERT_TRUE(controller.CanGoBack());
TestNavigationObserver go_back_observer(web_contents_impl);
controller.GoBack();
go_back_observer.Wait();
// These back/forward navigations do not involve a contents swap, since the
// original contents is gone as it was not adopted.
ASSERT_EQ(web_contents_impl, shell()->web_contents());
ASSERT_EQ(2, controller.GetEntryCount());
ASSERT_EQ(0, controller.GetLastCommittedEntryIndex());
EXPECT_EQ(main_url, controller.GetEntryAtIndex(0)->GetURL());
EXPECT_EQ(portal_url, controller.GetEntryAtIndex(1)->GetURL());
ASSERT_TRUE(controller.CanGoForward());
TestNavigationObserver go_forward_observer(web_contents_impl);
controller.GoForward();
go_forward_observer.Wait();
ASSERT_EQ(web_contents_impl, shell()->web_contents());
ASSERT_EQ(2, controller.GetEntryCount());
ASSERT_EQ(1, controller.GetLastCommittedEntryIndex());
EXPECT_EQ(main_url, controller.GetEntryAtIndex(0)->GetURL());
EXPECT_EQ(portal_url, controller.GetEntryAtIndex(1)->GetURL());
}
// Activation does not cancel new pending navigations in portals.
IN_PROC_BROWSER_TEST_F(PortalBrowserTest,
ActivationWithPortalPendingNavigation) {
GURL main_url(embedded_test_server()->GetURL("portal.test", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), main_url));
WebContentsImpl* web_contents_impl =
static_cast<WebContentsImpl*>(shell()->web_contents());
RenderFrameHostImpl* main_frame = web_contents_impl->GetMainFrame();
GURL portal_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
Portal* portal = CreatePortalToUrl(web_contents_impl, portal_url);
WebContentsImpl* portal_contents = portal->GetPortalContents();
NavigationControllerImpl& portal_controller =
portal_contents->GetController();
// Have the portal navigate so that we have a pending navigation.
GURL pending_url(embedded_test_server()->GetURL("a.com", "/title2.html"));
TestNavigationManager pending_navigation(portal_contents, pending_url);
EXPECT_TRUE(ExecJs(
main_frame,
JsReplace("document.querySelector('portal').src = $1;", pending_url)));
EXPECT_TRUE(pending_navigation.WaitForRequestStart());
// Navigating via frame proxy does not create a pending NavigationEntry. We'll
// check for an ongoing NavigationRequest instead.
FrameTreeNode* portal_node =
portal_contents->GetMainFrame()->frame_tree_node();
NavigationRequest* navigation_request = portal_node->navigation_request();
ASSERT_TRUE(navigation_request);
PortalActivatedObserver activated_observer(portal);
ASSERT_TRUE(
ExecJs(main_frame, "document.querySelector('portal').activate();"));
activated_observer.WaitForActivate();
NavigationControllerImpl& activated_controller = portal_controller;
pending_navigation.WaitForNavigationFinished();
ASSERT_EQ(2, activated_controller.GetEntryCount());
ASSERT_EQ(1, activated_controller.GetLastCommittedEntryIndex());
EXPECT_EQ(main_url, activated_controller.GetEntryAtIndex(0)->GetURL());
EXPECT_EQ(pending_url, activated_controller.GetEntryAtIndex(1)->GetURL());
}
class PortalOOPIFBrowserTest : public PortalBrowserTest {
protected:
PortalOOPIFBrowserTest() {}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment