PlzNavigate: fix regression on the FYI bot.

third_party/WebKit/Source/core/loader/FrameLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 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/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
@@ skipping 1443 matching lines @@
   if (m_progressTracker) {
     m_progressTracker->dispose();
     m_progressTracker.clear();
   }
 
   TRACE_EVENT_OBJECT_DELETED_WITH_ID("loading", "FrameLoader", this);
 }
 
 void FrameLoader::loadFailed(DocumentLoader* loader,
                              const ResourceError& error) {
+  m_isNavigationHandledByClient = false;

[Nate Chapin, 2017/01/11 20:30:46]

This isn't obvious to me. A blink-internal navigation failed, why would an
external navigation also fail?

[arthursonzogni, 2017/01/12 12:33:29]

If an external navigation is in progress, what other kind of load could fail? I
don't know how it works, but I expected that there could be only one thing to
load at a time.

If this is not a good idea to put it here, I can move this statement in the
upper level:
* 0) FrameLoader::loadFailed()
* 1) WebLocalFrameImpl::maybeRenderFallbackContent()
* 2) RenderFrameImpl::OnFailedNavigation()

But I will need to create functions to set m_isNavigationHandledByClient from
outside of the FrameLoader. Do you think to put this line of code in 1) is okay?

Note: In the future, I should try to remove the m_isNavigationHandledByClient as
soon as I understood better how the frameloader works and how it is connected to
the testrunner.

   if (!error.isCancellation() && m_frame->owner()) {
     // FIXME: For now, fallback content doesn't work cross process.
     if (m_frame->owner()->isLocal())
       m_frame->deprecatedLocalOwner()->renderFallbackContent();
   }
 
   HistoryCommitType historyCommitType = loadTypeToCommitType(m_loadType);
   if (loader == m_provisionalDocumentLoader) {
     m_provisionalDocumentLoader->setSentDidFinishLoad();
     client()->dispatchDidFailProvisionalLoad(error, historyCommitType);
     if (loader != m_provisionalDocumentLoader)
       return;
     detachDocumentLoader(m_provisionalDocumentLoader);
   } else {
     DCHECK_EQ(loader, m_documentLoader);
     if (m_frame->document()->parser())
       m_frame->document()->parser()->stopParsing();
     if (!m_documentLoader->sentDidFinishLoad()) {
       m_documentLoader->setSentDidFinishLoad();
       client()->dispatchDidFailLoad(error, historyCommitType);
     }
   }
   checkCompleted();

[arthursonzogni, 2017/01/10 13:24:25]

In https://codereview.chromium.org/2563423004 in FrameLoader::loadFailed()
The lines "m_progressTracker->progressCompleted()" were removed. It's not a
problem because in a non-PlzNavigate world, "checkCompleted()" would have called
it at the end if needed.

However, with PlzNavigate, checkCompleted is prevented to call it when
m_isNavigationHandledByClient is true. It become false again once
stopAllLoader() is called. I don't know if it is expected that stopAllLoader()
is not called?
Whatever happens, it is clear to me that m_isNavigationHandledByClient must be
false here because the browser-side navigation has failed and therefore has
finished.

 }
 
 bool FrameLoader::shouldPerformFragmentNavigation(bool isFormSubmission,
                                                   const String& httpMethod,
                                                   FrameLoadType loadType,
                                                   const KURL& url) {
   // We don't do this if we are submitting a form with method other than "GET",
   // explicitly reloading, currently displaying a frameset, or if the URL does
   // not have a fragment.
   return (!isFormSubmission || equalIgnoringCase(httpMethod, HTTPNames::GET)) &&
@@ skipping 399 matching lines @@
                          m_documentLoader ? m_documentLoader->url() : String());
   return tracedValue;
 }
 
 inline void FrameLoader::takeObjectSnapshot() const {
   TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID("loading", "FrameLoader", this,
                                       toTracedValue());
 }
 
 }  // namespace blink