Fix a race between navigation and an interstitial.

content/browser/frame_host/navigation_controller_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 1862 matching lines @@
   // will force the throbber to start, but WebKit will essentially ignore the
   // navigation, and won't send a message to stop the throbber. To prevent this
   // from happening, we drop the navigation here and stop the slow-to-commit
   // page from loading (which would normally happen during the navigation).
   if (pending_entry_index_ != -1 &&
       pending_entry_index_ == last_committed_entry_index_ &&
       pending_entry_->restore_type() == RestoreType::NONE &&
       pending_entry_->GetTransitionType() & ui::PAGE_TRANSITION_FORWARD_BACK) {
     delegate_->Stop();
 
-    // If an interstitial page is showing, we want to close it to get back
-    // to what was showing before.
-    if (delegate_->GetInterstitialPage())
-      delegate_->GetInterstitialPage()->DontProceed();
+    // If an interstitial page is showing, we want to close it to get back to
+    // what was showing before. There are two ways to get the interstitial page
+    // given a WebContents, but WebContents::GetInterstitialPage might return
+    // null if the interstitial was shown but not yet visible while

[Charlie Reis, 2017/06/22 16:19:38]

nit: s/shown/created/, perhaps?

[Avi (use Gerrit), 2017/06/22 18:08:51]

I'm talking about the range in which WebContents::GetInterstitialPage and
InterstitialPage::GetInterstitialPage differ. It's precisely the time from when
the interstitial was told to "show" through the time the interstitial is visible
that GetInterstitialPage returns null but GetInterstitialPage returns the
interstitial.

[Charlie Reis, 2017/06/22 21:06:00]

Ok.  Can you rephrase to say "if the interstitial's Show() method was called but
it is not yet visible, while..."?  I'm finding "shown but not visible" hard to
understand without clarifying that one of them is a method.

[Avi (use Gerrit), 2017/06/22 21:46:42]

I rewrote it a few times, and I'm now much happier with the phrasing. Thank you
for the push.

+    // InterstitialPage::GetInterstitialPage will return such an interstitial.
+    InterstitialPage* interstitial =
+        InterstitialPage::GetInterstitialPage(GetWebContents());
+    if (interstitial)
+      interstitial->DontProceed();
 
     DiscardNonCommittedEntries();
     return;
   }
 
   // If an interstitial page is showing, the previous renderer is blocked and
   // cannot make new requests.  Unblock (and disable) it to allow this
   // navigation to succeed.  The interstitial will stay visible until the
   // resulting DidNavigate.
   if (delegate_->GetInterstitialPage()) {
@@ skipping 337 matching lines @@
   DCHECK(pending_entry_index_ == -1 ||
          pending_entry_ == GetEntryAtIndex(pending_entry_index_));
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content