Add check for null pending entry in LoginTabHelper

In https://bugs.chromium.org/p/chromium/issues/detail?id=1005096, I
fixed a crash in HTTP Auth committed interstitials by changing a
GetVisibleEntry() to a GetPendingEntry(), and I reasoned that
GetPendingEntry() should always be non-null at this point. It turns
out, I reasoned wrong. It seems that there is some interleaving of
navigations starting and getting cancelled where the pending entry is
null when processing a main-frame response. So this CL adds a null
check before accessing the pending entry.

Bug: 1015787,1006955
Change-Id: I70e06e6c8aeee6db93c5797593d4eda6cd5c906d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1869299
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707602}

chrome/browser/ui/login/login_handler_unittest.cc

@@ -8,6 +8,9 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/login/login_handler.h"
+#include "chrome/browser/ui/login/login_tab_helper.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "content/public/test/mock_navigation_handle.h"
 #include "net/base/auth.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -111,6 +114,15 @@ base::string16 ExpectedAuthority(bool is_proxy, const char* prefix) {
   return str;
 }
 
+class LoginHandlerWithWebContentsTest : public ChromeRenderViewHostTestHarness {
+ public:
+  LoginHandlerWithWebContentsTest() {}
+  ~LoginHandlerWithWebContentsTest() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LoginHandlerWithWebContentsTest);
+};
+
 }  // namespace
 
 TEST(LoginHandlerTest, DialogStringsAndRealm) {
@@ -145,3 +157,15 @@ TEST(LoginHandlerTest, DialogStringsAndRealm) {
                  LoginHandler::GetSignonRealm(request_url, auth_info).c_str());
   }
 }
+
+// Tests that LoginTabHelper does not crash if
+// WillProcessMainFrameUnauthorizedResponse() is called when there is no pending
+// entry. Regression test for https://crbug.com/1015787.
+TEST_F(LoginHandlerWithWebContentsTest, NoPendingEntryDoesNotCrash) {
+  LoginTabHelper::CreateForWebContents(web_contents());
+  LoginTabHelper* helper = LoginTabHelper::FromWebContents(web_contents());
+  content::MockNavigationHandle handle;
+  content::NavigationThrottle::ThrottleCheckResult result =
+      helper->WillProcessMainFrameUnauthorizedResponse(&handle);
+  EXPECT_EQ(content::NavigationThrottle::CANCEL, result.action());
+}

chrome/browser/ui/login/login_tab_helper.cc

@@ -167,12 +167,9 @@ LoginTabHelper::WillProcessMainFrameUnauthorizedResponse(
   // commits. Comparing against GetVisibleEntry() would also work, but it's less
   // specific and not guaranteed to exist in all cases (e.g., in the case of
   // navigating a window just opened via window.open()).
-  //
-  // TODO(https://crbug.com/1006955): if this line is crashing, the assumption
-  // that GetPendingEntry() must be non-null is incorrect, in which case a null
-  // check should be added here.
-  if (web_contents()->GetController().GetPendingEntry()->GetUniqueID() ==
-      navigation_entry_id_with_cancelled_prompt_) {
+  if (web_contents()->GetController().GetPendingEntry() &&
+      web_contents()->GetController().GetPendingEntry()->GetUniqueID() ==
+          navigation_entry_id_with_cancelled_prompt_) {
     // Note the navigation handle ID so that when this refresh navigation
     // finishes, DidFinishNavigation declines to show another login prompt. We
     // need the navigation handle ID (rather than the navigation entry ID) here