Evict Surfaces from Before Navigation When Re-using DelegatedFrameHost

When we are not applying site-isolation a DelegatedFrameHost can be 
re-used when navigating between different pages. This can occur on 
Android as well as with the un-supported flag
--disable-site-isolation-trials.

When RenderWidgetHostImpl::ForceFirstFrameAfterNavigationTimeout is 
invoked, as either from tab-changing, or from thumbnailing, we set 
fallback surfaces. This is intended to be the first viz::SurfaceId that 
was generated by navigation.

However if a navigation fails we don't actually embed a new surface. The
DelegatedFrameHost then ends up utilizing outdated surfaces as the 
fallback.

This change updates DelegatedFrameHost to be notified of when a 
navigation begins. If we fail to embed a new surface by the timeout, 
we evict the surface that predates the navigation.

The ThumbnailTabHelper has been updated to also clear its currently 
cached thumbnail in the case that the page has transitioned back to the
unready state. Thus clearing thumbnails that exist from before a 
navigation.

TEST=NoCompositingRenderWidgetHostViewBrowserTest.
         NoFallbackAfterHiddenNavigationFails

Bug: 1152894
Change-Id: Ia5734abb5201a56c271114a891bc81212a3aa975
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585790Reviewed-by: Dana Fried <dfried@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837779}

chrome/browser/ui/thumbnails/thumbnail_image.cc

@@ -81,6 +81,16 @@ void ThumbnailImage::AssignSkBitmap(SkBitmap bitmap) {
                      weak_ptr_factory_.GetWeakPtr(), base::TimeTicks::Now()));
 }
 
+void ThumbnailImage::ClearData() {
+  // TODO(collinbaker): Update this to notify the observers that the data has
+  // changed. It may be necessary to re-engineer them to accept empty/invalid
+  // data. crbug.com/1152894
+  if (!data_)
+    return;
+  data_->data.clear();
+  data_.reset();
+}
+
 void ThumbnailImage::RequestThumbnailImage() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   ConvertJPEGDataToImageSkiaAndNotifyObservers();

chrome/browser/ui/thumbnails/thumbnail_image.h

@@ -84,6 +84,10 @@ class ThumbnailImage : public base::RefCounted<ThumbnailImage> {
   // Sets the SkBitmap data and notifies observers with the resulting image.
   void AssignSkBitmap(SkBitmap bitmap);
 
+  // Clears the currently set |data_|, for when the current thumbnail is no
+  // longer valid to display.
+  void ClearData();
+
   // Requests that a thumbnail image be made available to observers. Does not
   // guarantee that Observer::OnThumbnailImageAvailable() will be called, or how
   // long it will take, though in most cases it should happen very quickly.

chrome/browser/ui/thumbnails/thumbnail_tab_helper.cc

@@ -183,6 +183,12 @@ class ThumbnailTabHelper::TabStateTracker
   }
 
   void PageReadinessChanged(PageReadiness readiness) {
+    if (page_readiness_ == readiness)
+      return;
+    // If we transition back to a kNotReady state, clear any existing thumbnail,
+    // as it will contain an old snapshot, possibly from a different domain.
+    if (readiness == PageReadiness::kNotReady)
+      thumbnail_tab_helper_->ClearData();
     page_readiness_ = readiness;
     capture_driver_.UpdatePageReadiness(readiness);
   }
@@ -272,16 +278,22 @@ void ThumbnailTabHelper::StoreThumbnailForTabSwitch(base::TimeTicks start_time,
 
 void ThumbnailTabHelper::StoreThumbnail(CaptureType type,
                                         const SkBitmap& bitmap) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
+  // Failed requests will return an empty bitmap. In tests this can be triggered
+  // on threads other than the UI thread.
   if (bitmap.drawsNothing())
     return;
 
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
   RecordCaptureType(type);
   state_->OnFrameCaptured(type);
   thumbnail_->AssignSkBitmap(bitmap);
 }
 
+void ThumbnailTabHelper::ClearData() {
+  thumbnail_->ClearData();
+}
+
 void ThumbnailTabHelper::StartVideoCapture() {
   content::RenderWidgetHostView* const source_view = state_->GetView();
   if (!source_view)

chrome/browser/ui/thumbnails/thumbnail_tab_helper.h

@@ -51,6 +51,9 @@ class ThumbnailTabHelper
   void StoreThumbnailForTabSwitch(base::TimeTicks start_time,
                                   const SkBitmap& bitmap);
   void StoreThumbnail(CaptureType type, const SkBitmap& bitmap);
+  // Clears the data associated to the currently set thumbnail. For when the
+  // thumbnail is no longer valid.
+  void ClearData();
 
   // viz::mojom::FrameSinkVideoConsumer:
 

content/browser/renderer_host/delegated_frame_host.cc

@@ -150,11 +150,15 @@ void DelegatedFrameHost::CopyFromCompositingSurfaceInternal(
     const gfx::Size& output_size,
     viz::CopyOutputRequest::ResultFormat format,
     viz::CopyOutputRequest::CopyOutputRequestCallback callback) {
-  DCHECK(CanCopyFromCompositingSurface());
-
   auto request =
       std::make_unique<viz::CopyOutputRequest>(format, std::move(callback));
 
+  // It is possible for us to not have a valid surface to copy from. Such as
+  // if a navigation fails to complete. In such a case do not attempt to request
+  // a copy.
+  if (!CanCopyFromCompositingSurface())
+    return;
+
   if (!src_subrect.IsEmpty()) {
     request->set_area(
         gfx::ScaleToRoundedRect(src_subrect, client_->GetDeviceScaleFactor()));
@@ -220,6 +224,16 @@ void DelegatedFrameHost::EmbedSurface(
   local_surface_id_ = new_local_surface_id;
   surface_dip_size_ = new_dip_size;
 
+  // The embedding of a new surface completes the navigation process.
+  pre_navigation_local_surface_id_ = viz::LocalSurfaceId();
+
+  // Navigations performed while hidden delay embedding until transitioning to
+  // becoming visible. So we may not have a valid surace when DidNavigate is
+  // called. Cache the first surface here so we have the correct oldest surface
+  // to fallback to.
+  if (!first_local_surface_id_after_navigation_.is_valid())
+    first_local_surface_id_after_navigation_ = local_surface_id_;
+
   viz::SurfaceId new_primary_surface_id(frame_sink_id_, local_surface_id_);
 
   if (!client_->DelegatedFrameHostIsVisible()) {
@@ -296,8 +310,17 @@ void DelegatedFrameHost::ResetFallbackToFirstNavigationSurface() {
     return;
   }
 
+  // We never completed navigation, evict our surfaces.
+  if (pre_navigation_local_surface_id_.is_valid() &&
+      !first_local_surface_id_after_navigation_.is_valid()) {
+    EvictDelegatedFrame();
+  }
+
   client_->DelegatedFrameHostGetLayer()->SetOldestAcceptableFallback(
-      viz::SurfaceId(frame_sink_id_, first_local_surface_id_after_navigation_));
+      first_local_surface_id_after_navigation_.is_valid()
+          ? viz::SurfaceId(frame_sink_id_,
+                           first_local_surface_id_after_navigation_)
+          : viz::SurfaceId());
 }
 
 void DelegatedFrameHost::EvictDelegatedFrame() {
@@ -377,13 +400,20 @@ void DelegatedFrameHost::ContinueDelegatedFrameEviction() {
   if (!HasSavedFrame())
     return;
 
-  DCHECK(!client_->DelegatedFrameHostIsVisible());
   std::vector<viz::SurfaceId> surface_ids = {
       client_->CollectSurfaceIdsForEviction()};
+
+  // If we have a surface from before a navigation, evict it as well.
+  if (pre_navigation_local_surface_id_.is_valid()) {
+    viz::SurfaceId id(frame_sink_id_, pre_navigation_local_surface_id_);
+    surface_ids.push_back(id);
+  }
+
   // This list could be empty if this frame is not in the frame tree (can happen
   // during navigation, construction, destruction, or in unit tests).
   if (!surface_ids.empty()) {
-    DCHECK(std::find(surface_ids.begin(), surface_ids.end(),
+    DCHECK(!GetCurrentSurfaceId().is_valid() ||
+           std::find(surface_ids.begin(), surface_ids.end(),
                      GetCurrentSurfaceId()) != surface_ids.end());
     DCHECK(host_frame_sink_manager_);
     host_frame_sink_manager_->EvictSurfaces(surface_ids);
@@ -427,6 +457,18 @@ void DelegatedFrameHost::DidNavigate() {
   first_local_surface_id_after_navigation_ = local_surface_id_;
 }
 
+void DelegatedFrameHost::OnNavigateToNewPage() {
+  // We are navigating to a different page, so the current |local_surface_id_|
+  // and the fallback option of |first_local_surface_id_after_navigation_| are
+  // no longer valid, as they represent older content from a different source.
+  //
+  // Cache the current |local_surface_id_| so that if navigation fails we can
+  // evict it when transitioning to becoming visible.
+  pre_navigation_local_surface_id_ = local_surface_id_;
+  first_local_surface_id_after_navigation_ = viz::LocalSurfaceId();
+  local_surface_id_ = viz::LocalSurfaceId();
+}
+
 void DelegatedFrameHost::WindowTitleChanged(const std::string& title) {
   if (host_frame_sink_manager_)
     host_frame_sink_manager_->SetFrameSinkDebugLabel(frame_sink_id_, title);

content/browser/renderer_host/delegated_frame_host.h

@@ -152,6 +152,10 @@ class CONTENT_EXPORT DelegatedFrameHost
   }
 
   void DidNavigate();
+  // Navigation to a different page than the current one has begun. Caches the
+  // current LocalSurfaceId information so that old content can be evicted if
+  // navigation fails to complete.
+  void OnNavigateToNewPage();
 
   void WindowTitleChanged(const std::string& title);
 
@@ -219,6 +223,10 @@ class CONTENT_EXPORT DelegatedFrameHost
   std::unique_ptr<viz::FrameEvictor> frame_evictor_;
 
   viz::LocalSurfaceId first_local_surface_id_after_navigation_;
+  // While navigating we have no active |local_surface_id_|. Track the one from
+  // before a navigation, because if the navigation fails to complete, we will
+  // need to evict its surface.
+  viz::LocalSurfaceId pre_navigation_local_surface_id_;
 
   FrameEvictionState frame_eviction_state_ = FrameEvictionState::kNotStarted;
 

content/browser/renderer_host/render_widget_host_view_android.cc

@@ -978,6 +978,8 @@ void RenderWidgetHostViewAndroid::OnDidNavigateMainFrameToNewPage() {
   if (view_.parent())
     view_.parent()->MoveToFront(&view_);
   ResetGestureDetection();
+  if (delegated_frame_host_)
+    delegated_frame_host_->OnNavigateToNewPage();
 }
 
 void RenderWidgetHostViewAndroid::SetDoubleTapSupportEnabled(bool enabled) {

content/browser/renderer_host/render_widget_host_view_aura.cc

@@ -2379,6 +2379,10 @@ void RenderWidgetHostViewAura::CreateSelectionController() {
 }
 
 void RenderWidgetHostViewAura::OnDidNavigateMainFrameToNewPage() {
+  // Invalidate the surface so that we don't attempt to evict it multiple times.
+  window_->InvalidateLocalSurfaceId();
+  if (delegated_frame_host_)
+    delegated_frame_host_->OnNavigateToNewPage();
   CancelActiveTouches();
 }
 

content/browser/renderer_host/render_widget_host_view_base.h

@@ -593,6 +593,8 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView {
   FRIEND_TEST_ALL_PREFIXES(DelegatedInkPointTest, EventForwardedToCompositor);
   FRIEND_TEST_ALL_PREFIXES(DelegatedInkPointTest,
                            MojoInterfaceReboundOnDisconnect);
+  FRIEND_TEST_ALL_PREFIXES(NoCompositingRenderWidgetHostViewBrowserTest,
+                           NoFallbackAfterHiddenNavigationFails);
 
   void SynchronizeVisualProperties();
 

content/browser/renderer_host/render_widget_host_view_browsertest.cc

@@ -15,6 +15,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
+#include "cc/layers/surface_layer.h"
 #include "content/browser/gpu/compositor_util.h"
 #include "content/browser/renderer_host/dip_util.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -61,6 +62,8 @@ namespace {
     return;  \
   }
 
+}  // namespace
+
 // Common base class for browser tests.  This is subclassed three times: Once to
 // test the browser in forced-compositing mode; once to test with compositing
 // mode disabled; once with no surface creation for non-visual tests.
@@ -274,10 +277,14 @@ IN_PROC_BROWSER_TEST_F(NoCompositingRenderWidgetHostViewBrowserTest,
 
 #if defined(OS_ANDROID)
   // Navigating while hidden should not generate a new surface. As the old one
-  // is maintained as the fallback.
+  // is maintained as the fallback. The DelegatedFrameHost should have not have
+  // a valid active viz::LocalSurfaceId until the first surface after navigation
+  // has been embedded.
   EXPECT_TRUE(dfh->HasPrimarySurface());
   EXPECT_FALSE(dfh->IsPrimarySurfaceEvicted());
-  EXPECT_EQ(initial_local_surface_id, dfh->SurfaceId().local_surface_id());
+  EXPECT_EQ(initial_local_surface_id,
+            dfh->content_layer_for_testing()->surface_id().local_surface_id());
+  EXPECT_FALSE(dfh->SurfaceId().local_surface_id().is_valid());
 #endif
 
   // Showing the view should lead to a new surface being embedded.
@@ -294,6 +301,84 @@ IN_PROC_BROWSER_TEST_F(NoCompositingRenderWidgetHostViewBrowserTest,
   EXPECT_NE(initial_local_surface_id, new_local_surface_id);
 #endif
 }
+
+// Tests that if navigation fails, when re-using a RenderWidgetHostViewBase, and
+// while it is hidden, that the fallback surface if invalidated. Then that when
+// becoming visible, that a new valid surface is produced.
+IN_PROC_BROWSER_TEST_F(NoCompositingRenderWidgetHostViewBrowserTest,
+                       NoFallbackAfterHiddenNavigationFails) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  // Creates the initial RenderWidgetHostViewBase, and connects to a
+  // CompositorFrameSink.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("/page_with_animation.html")));
+  RenderWidgetHostViewBase* rwhvb = GetRenderWidgetHostView();
+  ASSERT_TRUE(rwhvb);
+  viz::LocalSurfaceId rwhvb_local_surface_id = rwhvb->GetLocalSurfaceId();
+  EXPECT_TRUE(rwhvb_local_surface_id.is_valid());
+
+  // Hide the view before performing the next navigation.
+  shell()->web_contents()->WasHidden();
+#if defined(OS_ANDROID)
+  // On Android we want to ensure that we maintain the currently embedded
+  // surface. So that there is something to display when returning to the tab.
+  RenderWidgetHostViewAndroid* rwhva =
+      static_cast<RenderWidgetHostViewAndroid*>(rwhvb);
+  ui::DelegatedFrameHostAndroid* dfh =
+      rwhva->delegated_frame_host_for_testing();
+  EXPECT_TRUE(dfh->HasPrimarySurface());
+  EXPECT_FALSE(dfh->IsPrimarySurfaceEvicted());
+  viz::LocalSurfaceId initial_local_surface_id =
+      dfh->SurfaceId().local_surface_id();
+  EXPECT_TRUE(initial_local_surface_id.is_valid());
+#endif
+
+  // Perform a navigation to the same content source. This will reuse the
+  // existing RenderWidgetHostViewBase.
+  EXPECT_TRUE(NavigateToURL(
+      shell(), embedded_test_server()->GetURL("/page_with_animation.html")));
+  EXPECT_FALSE(rwhvb->GetLocalSurfaceId().is_valid());
+
+  // Surface Synchronization can lead to several different Surfaces being
+  // embedded during a navigation. Ending once the Browser and Renderer have
+  // agreed to a set of VisualProperties.
+  //
+  // If this takes too long we hit a timeout that attempts to reset us back to
+  // the initial surface. So that some content state can be presented.
+  //
+  // If a navigation were to fail, then this would be invoked before any new
+  // surface is embedded. For which we expect it to clear out the fallback
+  // surfaces. As we cannot fallback to a surface from before navigation.
+  rwhvb->ResetFallbackToFirstNavigationSurface();
+  EXPECT_FALSE(rwhvb->HasFallbackSurface());
+
+#if defined(OS_ANDROID)
+  // Navigating while hidden should not generate a new surface.
+  // The failed navigation above will lead to the primary surface being evicted.
+  // The DelegatedFrameHost should have not have a valid active
+  // viz::LocalSurfaceId until the first surface after navigation has been
+  // embedded.
+  EXPECT_FALSE(dfh->HasPrimarySurface());
+  EXPECT_TRUE(dfh->IsPrimarySurfaceEvicted());
+  EXPECT_FALSE(dfh->content_layer_for_testing()->surface_id().is_valid());
+  EXPECT_FALSE(dfh->SurfaceId().local_surface_id().is_valid());
+#endif
+
+  // Showing the view should lead to a new surface being embedded.
+  shell()->web_contents()->WasShown();
+  viz::LocalSurfaceId new_rwhvb_local_surface_id = rwhvb->GetLocalSurfaceId();
+  EXPECT_TRUE(new_rwhvb_local_surface_id.is_valid());
+  EXPECT_NE(rwhvb_local_surface_id, new_rwhvb_local_surface_id);
+#if defined(OS_ANDROID)
+  EXPECT_TRUE(dfh->HasPrimarySurface());
+  EXPECT_FALSE(dfh->IsPrimarySurfaceEvicted());
+  viz::LocalSurfaceId new_local_surface_id =
+      dfh->SurfaceId().local_surface_id();
+  EXPECT_TRUE(new_local_surface_id.is_valid());
+  EXPECT_NE(initial_local_surface_id, new_local_surface_id);
+#endif
+}
+
 #endif  // !defined(OS_MAC)
 
 namespace {
@@ -896,5 +981,4 @@ INSTANTIATE_TEST_SUITE_P(
 
 #endif  // !defined(OS_ANDROID)
 
-}  // namespace
 }  // namespace content

ui/android/delegated_frame_host_android.cc

@@ -158,10 +158,24 @@ bool DelegatedFrameHostAndroid::CanCopyFromCompositingSurface() const {
 void DelegatedFrameHostAndroid::EvictDelegatedFrame() {
   content_layer_->SetSurfaceId(viz::SurfaceId(),
                                cc::DeadlinePolicy::UseDefaultDeadline());
-  if (!HasSavedFrame() || frame_evictor_->visible())
+  std::vector<viz::SurfaceId> surface_ids;
+  // If we have a surface from before a navigation, evict it, regardless of
+  // visibility state.
+  if (pre_navigation_local_surface_id_.is_valid()) {
+    viz::SurfaceId pre_nav =
+        viz::SurfaceId(frame_sink_id_, pre_navigation_local_surface_id_);
+    surface_ids.push_back(pre_nav);
+  } else if (!HasSavedFrame() || frame_evictor_->visible()) {
+    return;
+  }
+
+  if (local_surface_id_.is_valid()) {
+    viz::SurfaceId current = viz::SurfaceId(frame_sink_id_, local_surface_id_);
+    surface_ids.push_back(current);
+  }
+
+  if (surface_ids.empty())
     return;
-  std::vector<viz::SurfaceId> surface_ids = {
-      viz::SurfaceId(frame_sink_id_, local_surface_id_)};
   host_frame_sink_manager_->EvictSurfaces(surface_ids);
   frame_evictor_->OnSurfaceDiscarded();
   // When surface sync is on, this call will force |client_| to allocate a new
@@ -183,6 +197,14 @@ void DelegatedFrameHostAndroid::ResetFallbackToFirstNavigationSurface() {
           .IsSameOrNewerThan(first_local_surface_id_after_navigation_)) {
     return;
   }
+
+  // If we have a surface from before a navigation, evict it as well.
+  if (pre_navigation_local_surface_id_.is_valid() &&
+      !first_local_surface_id_after_navigation_.is_valid()) {
+    EvictDelegatedFrame();
+    content_layer_->SetBackgroundColor(SK_ColorTRANSPARENT);
+  }
+
   content_layer_->SetOldestAcceptableFallback(
       viz::SurfaceId(frame_sink_id_, first_local_surface_id_after_navigation_));
 }
@@ -246,7 +268,20 @@ void DelegatedFrameHostAndroid::EmbedSurface(
   // at serialization time.
   CHECK(new_local_surface_id.is_valid());
 
+  // Confirm that there is a valid fallback surface on, otherwise we need to
+  // adjust deadline times. To avoid displaying invalid content.
+  bool has_fallback_surface =
+      (content_layer_->oldest_acceptable_fallback() &&
+       content_layer_->oldest_acceptable_fallback()->is_valid());
   local_surface_id_ = new_local_surface_id;
+  // The embedding of a new surface completes the navigation process.
+  pre_navigation_local_surface_id_ = viz::LocalSurfaceId();
+  // Navigations performed while hidden delay embedding until transitioning to
+  // becoming visible. So we may not have a valid surace when DidNavigate is
+  // called. Cache the first surface here so we have the correct oldest surface
+  // to fallback to.
+  if (!first_local_surface_id_after_navigation_.is_valid())
+    first_local_surface_id_after_navigation_ = local_surface_id_;
   surface_size_in_pixels_ = new_size_in_pixels;
 
   viz::SurfaceId current_primary_surface_id = content_layer_->surface_id();
@@ -258,8 +293,7 @@ void DelegatedFrameHostAndroid::EmbedSurface(
     // screen if renderer won't submit frame in time. See
     // https://crbug.com/1088369 and  https://crbug.com/813157
     if (surface_size_in_pixels_ != content_layer_->bounds() &&
-        content_layer_->oldest_acceptable_fallback() &&
-        content_layer_->oldest_acceptable_fallback()->is_valid()) {
+        has_fallback_surface) {
       content_layer_->SetOldestAcceptableFallback(new_primary_surface_id);
 
       // We default to black background for fullscreen case.
@@ -293,6 +327,11 @@ void DelegatedFrameHostAndroid::EmbedSurface(
         deadline_policy = cc::DeadlinePolicy::UseSpecifiedDeadline(0u);
       }
     }
+    // If there is not a valid current surface, nor a valid fallback, we want to
+    // produce new content as soon as possible. To avoid displaying invalide
+    // content, such as surfaces from before a navigation.
+    if (!has_fallback_surface)
+      deadline_policy = cc::DeadlinePolicy::UseSpecifiedDeadline(0u);
     content_layer_->SetSurfaceId(new_primary_surface_id, deadline_policy);
     content_layer_->SetBounds(new_size_in_pixels);
   }
@@ -343,6 +382,18 @@ void DelegatedFrameHostAndroid::DidNavigate() {
   first_local_surface_id_after_navigation_ = local_surface_id_;
 }
 
+void DelegatedFrameHostAndroid::OnNavigateToNewPage() {
+  // We are navigating to a different page, so the current |local_surface_id_|
+  // and the fallback option of |first_local_surface_id_after_navigation_| are
+  // no longer valid, as they represent older content from a different source.
+  //
+  // Cache the current |local_surface_id_| so that if navigation fails we can
+  // evict it when transitioning to becoming visible.
+  pre_navigation_local_surface_id_ = local_surface_id_;
+  first_local_surface_id_after_navigation_ = viz::LocalSurfaceId();
+  local_surface_id_ = viz::LocalSurfaceId();
+}
+
 void DelegatedFrameHostAndroid::SetTopControlsVisibleHeight(float height) {
   if (top_controls_visible_height_ == height)
     return;

ui/android/delegated_frame_host_android.h

@@ -121,7 +121,15 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
 
   void TakeFallbackContentFrom(DelegatedFrameHostAndroid* other);
 
+  // Called when navigation has completed, and this DelegatedFrameHost is
+  // visible. A new Surface will have been embedded at this point. If navigation
+  // is done while hidden, this will be called upon becoming visible.
   void DidNavigate();
+  // Navigation to a different page than the current one has begun. This is
+  // called regardless of the visibility of the page. Caches the current
+  // LocalSurfaceId information so that old content can be evicted if
+  // navigation fails to complete.
+  void OnNavigateToNewPage();
 
   void SetTopControlsVisibleHeight(float height);
 
@@ -151,6 +159,10 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid
   // Whether we've received a frame from the renderer since navigating.
   // Only used when surface synchronization is on.
   viz::LocalSurfaceId first_local_surface_id_after_navigation_;
+  // While navigating we have no active |local_surface_id_|. Track the one from
+  // before a navigation, because if the navigation fails to complete, we will
+  // need to evict its surface.
+  viz::LocalSurfaceId pre_navigation_local_surface_id_;
 
   // The LocalSurfaceId of the currently embedded surface. If surface sync is
   // on, this surface is not necessarily active.