[WIP] Fix history.replaceState() not propagating back favicons

components/history/core/browser/history_backend.cc

 // Copyright (c) 2012 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.
 
 #include "components/history/core/browser/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
@@ skipping 454 matching lines @@
 
   // If the user is adding older history, we need to make sure our times
   // are correct.
   if (request.time < first_recorded_time_)
     first_recorded_time_ = request.time;
 
   ui::PageTransition request_transition = request.transition;
   bool is_keyword_generated = ui::PageTransitionCoreTypeIs(
       request_transition, ui::PAGE_TRANSITION_KEYWORD_GENERATED);
 
+  RedirectList redirects = request.redirects;

[sky, 2017/06/22 15:26:39]

This is worth a comment.
Also, avoid the potential unnecessary copy of redirects here. By that I mean do
something like:

RedirectList redirects;
if (your-new-code) {
} else {
  redirects = request.redirects;
}

+  if (request.replaced_entry_url.has_value()) {
+    RedirectList prior_redirects;
+    GetCachedRecentRedirects(*request.replaced_entry_url, &prior_redirects);
+    prior_redirects.insert(prior_redirects.end(), redirects.begin(),
+                           redirects.end());
+    redirects = std::move(prior_redirects);
+  }
+
   // If the user is navigating to a not-previously-typed intranet hostname,
   // change the transition to TYPED so that the omnibox will learn that this is
   // a known host.
-  bool has_redirects = request.redirects.size() > 1;
+  bool has_redirects = redirects.size() > 1;
   if (ui::PageTransitionIsMainFrame(request_transition) &&
       !ui::PageTransitionCoreTypeIs(request_transition,
                                     ui::PAGE_TRANSITION_TYPED) &&
       !is_keyword_generated) {
     // Check both the start and end of a redirect chain, since the user will
     // consider both to have been "navigated to".
     if (IsUntypedIntranetHost(request.url) ||
-        (has_redirects && IsUntypedIntranetHost(request.redirects[0]))) {
+        (has_redirects && IsUntypedIntranetHost(redirects[0]))) {
       request_transition = ui::PageTransitionFromInt(
           ui::PAGE_TRANSITION_TYPED |
           ui::PageTransitionGetQualifier(request_transition));
     }
   }
 
   if (!has_redirects) {
     // The single entry is both a chain start and end.
     ui::PageTransition t = ui::PageTransitionFromInt(
         request_transition | ui::PAGE_TRANSITION_CHAIN_START |
@@ skipping 11 matching lines @@
                      request.time);
 
       // Update the referrer's duration.
       UpdateVisitDuration(from_visit_id, request.time);
     }
   } else {
     // Redirect case. Add the redirect chain.
 
     ui::PageTransition redirect_info = ui::PAGE_TRANSITION_CHAIN_START;
 
-    RedirectList redirects = request.redirects;
     if (redirects[0].SchemeIs(url::kAboutScheme)) {
       // When the redirect source + referrer is "about" we skip it. This
       // happens when a page opens a new frame/window to about:blank and then
       // script sets the URL to somewhere else (used to hide the referrer). It
       // would be nice to keep all these redirects properly but we don't ever
       // see the initial about:blank load, so we don't know where the
       // subsequent client redirect came from.
       //
       // In this case, we just don't bother hooking up the source of the
       // redirects, so we remove it.
@@ skipping 11 matching lines @@
       // chain.
       if (request.referrer.is_valid()) {
         DCHECK_EQ(request.referrer, redirects[0]);
         redirects.erase(redirects.begin());
 
         // If the navigation entry for this visit has replaced that for the
         // first visit, remove the CHAIN_END marker from the first visit. This
         // can be called a lot, for example, the page cycler, and most of the
         // time we won't have changed anything.
         VisitRow visit_row;
-        if (request.did_replace_entry &&
+        if (request.replaced_entry_url.has_value() &&
             db_->GetRowForVisit(last_ids.second, &visit_row) &&
             visit_row.transition & ui::PAGE_TRANSITION_CHAIN_END) {
           visit_row.transition = ui::PageTransitionFromInt(
               visit_row.transition & ~ui::PAGE_TRANSITION_CHAIN_END);
           db_->UpdateVisitRow(visit_row);
         }
       }
     }
 
     for (size_t redirect_index = 0; redirect_index < redirects.size();
@@ skipping 1518 matching lines @@
     const GURL& page_url,
     favicon_base::IconType icon_type,
     const std::vector<favicon_base::FaviconID>& icon_ids) {
   if (!thumbnail_db_)
     return false;
 
   // Find all the pages whose favicons we should set, we want to set it for
   // all the pages in the redirect chain if it redirected.
   RedirectList redirects;
   GetCachedRecentRedirects(page_url, &redirects);
-  bool mappings_changed = SetFaviconMappingsForPages(redirects, icon_type,
-                                                     icon_ids);
-  if (page_url.has_ref()) {
-    // Refs often gets added by Javascript, but the redirect chain is keyed to
-    // the URL without a ref.
-    GURL::Replacements replacements;
-    replacements.ClearRef();
-    GURL page_url_without_ref = page_url.ReplaceComponents(replacements);
-    GetCachedRecentRedirects(page_url_without_ref, &redirects);
-    mappings_changed |= SetFaviconMappingsForPages(redirects, icon_type,
-                                                   icon_ids);
-  }
 
-  return mappings_changed;
+  return SetFaviconMappingsForPages(redirects, icon_type, icon_ids);
 }
 
 bool HistoryBackend::SetFaviconMappingsForPages(
     const std::vector<GURL>& page_urls,
     favicon_base::IconType icon_type,
     const std::vector<favicon_base::FaviconID>& icon_ids) {
   bool mappings_changed = false;
   for (auto i(page_urls.begin()); i != page_urls.end(); ++i) {
     mappings_changed |= SetFaviconMappingsForPage(*i, icon_type, icon_ids);
   }
@@ skipping 526 matching lines @@
   // transaction is currently open.
   db_->CommitTransaction();
   db_->Vacuum();
   db_->BeginTransaction();
   db_->GetStartDate(&first_recorded_time_);
 
   return true;
 }
 
 }  // namespace history