Use URL without fragment when query recent redirect list for favicons.

components/history/core/browser/history_backend.cc

Index: components/history/core/browser/history_backend.cc
diff --git a/components/history/core/browser/history_backend.cc b/components/history/core/browser/history_backend.cc
index d04c038d5bef7fd7c6d97a6bd878e7288a5b94ef..8fcc4741a041d0adad5266f1577af6b2a9417267 100644
--- a/components/history/core/browser/history_backend.cc
+++ b/components/history/core/browser/history_backend.cc
@@ -2085,12 +2085,28 @@ bool HistoryBackend::SetFaviconMappingsForPageAndRedirects(
   // 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);
+    mappings_changed = mappings_changed ||

[sky, 2015/06/17 17:39:27]

Don't you need the |= here, otherwise if mappings_changed is already true this
won't get executed, right?

[beaudoin, 2015/06/17 18:06:39]

That's what I do but since ||= doesnt exist I do m_c = m_c || ...

Is it OK to use bitwise |= on booleans?

[sky, 2015/06/17 21:20:45]

The way you have the code here SetFavicon... is not called if mapping_changed is
true (try it on windows). If you use a bitwire operator it will always be
called. This is why the code on 2110 uses a bitwire operator.

[beaudoin, 2015/06/17 22:50:29]

Ack.

+        SetFaviconMappingsForPages(redirects, icon_type, icon_ids);
+  }
 
-  bool mappings_changed = false;
+  return mappings_changed;
+}
 
-  // Save page <-> favicon associations.
-  for (RedirectList::const_iterator i(redirects.begin()); i != redirects.end();
-       ++i) {
+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 (std::vector<GURL>::const_iterator i(page_urls.begin());

[sky, 2015/06/17 17:39:27]

I would use auto here, but this is ok.

[beaudoin, 2015/06/17 18:06:39]

Done.

+       i != page_urls.end(); ++i) {
     mappings_changed |= SetFaviconMappingsForPage(*i, icon_type, icon_ids);
   }
   return mappings_changed;