PlzNavigate: Clear provisional history item on redirects.

third_party/WebKit/Source/web/WebDataSourceImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 57 matching lines @@
 WebURL WebDataSourceImpl::unreachableURL() const
 {
     return DocumentLoader::unreachableURL();
 }
 
 void WebDataSourceImpl::appendRedirect(const WebURL& url)
 {
     DocumentLoader::appendRedirect(url);
 }
 
-void WebDataSourceImpl::updateNavigationTimings(double redirectStartTime, double redirectEndTime, double fetchStartTime, const WebVector<WebURL>& redirectChain)
+void WebDataSourceImpl::updateNavigation(double redirectStartTime, double redirectEndTime, double fetchStartTime, const WebVector<WebURL>& redirectChain)

[Charlie Reis, 2016/08/09 05:07:31]

It's not clear to me whether this is the right place to put the check, so maybe
Nate can weigh in here.

[arthursonzogni, 2016/08/10 10:02:24]

See another reply above.

 {
     for (size_t i = 0; i + 1 < redirectChain.size(); ++i)
         timing().addRedirect(redirectChain[i], redirectChain[i + 1]);
     timing().setRedirectStart(redirectStartTime);
     timing().setRedirectEnd(redirectEndTime);
     timing().setFetchStart(fetchStartTime);
+
+    // If a redirection happens during a back/forward navigation, don't reuse
+    // any state from the previous HistoryItem

[Charlie Reis, 2016/08/09 05:07:31]

nit: End comment sentences with a period.

[arthursonzogni, 2016/08/10 10:02:24]

Done.

+    if (redirectChain.size() >= 2)

[nasko, 2016/08/03 16:31:25]

How do we know this is a back/forward navigation here? Wouldn't this be hit by
just doing a regular navigation with redirects?

[arthursonzogni, 2016/08/04 14:04:53]

Yes it is. But from what I understood, m_provisionalItem is non-NULL and used
only when the navigation is back/forward. Thus, during regular navigation the
provisionalItem is already NULL. 

But I will add japhet@ and creis@ to be sure.

[Charlie Reis, 2016/08/09 05:07:31]

Right.  That seems consistent with how FrameLoader::receivedMainResourceRedirect
works, and it looks like FrameLoader only sets m_provisionalItem for
back/forwards.

+        discardProvisionalHistoryItem();
 }
 
 void WebDataSourceImpl::redirectChain(WebVector<WebURL>& result) const
 {
     result.assign(m_redirectChain);
 }
 
 bool WebDataSourceImpl::isClientRedirect() const
 {
     return DocumentLoader::isClientRedirect();
@@ skipping 68 matching lines @@
 {
     DocumentLoader::setSubresourceFilter(WTF::wrapUnique(subresourceFilter));
 }
 
 DEFINE_TRACE(WebDataSourceImpl)
 {
     DocumentLoader::trace(visitor);
 }
 
 } // namespace blink