favicon: Replace usage of GetActiveEntry by GetLastCommittedEntry.

components/favicon/content/content_favicon_driver.cc

 // Copyright 2015 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/favicon/content/content_favicon_driver.h"
 
 #include "base/bind.h"
 #include "components/favicon/content/favicon_url_util.h"
 #include "components/favicon/core/favicon_url.h"
 #include "content/public/browser/browser_context.h"
@@ skipping 64 matching lines @@
       url, true, max_image_size, bypass_cache,
       base::Bind(&FaviconDriverImpl::DidDownloadFavicon,
                  base::Unretained(this)));
 }
 
 bool ContentFaviconDriver::IsOffTheRecord() {
   DCHECK(web_contents());
   return web_contents()->GetBrowserContext()->IsOffTheRecord();
 }
 
 GURL ContentFaviconDriver::GetActiveURL() {

[tfarina, 2015/06/30 21:22:55]

Nasko, I guess you want me to rename this and GetActiveTitle()?

[tfarina, 2015/06/30 21:22:55]

Should I rename it to GetLastCommittedURL()?

[nasko, 2015/07/01 05:56:30]

Ideally, this should be renamed, but there are many more methods named Active.
I'm not sure if renaming all methods to be consistent is something we should do.
I'll leave it up to the component owner to decide.

   content::NavigationEntry* entry =
-      web_contents()->GetController().GetActiveEntry();
+      web_contents()->GetController().GetLastCommittedEntry();

[nasko, 2015/07/01 05:56:30]

Let's watch for failures/bugs from this CL once it lands. It is possible that
favicon and title are expected to be updated on back/forward navigation prior to
the commit of it. If that is the case, we will have to use GetVisibleEntry.

[sdefresne, 2015/08/13 09:12:26]

drive-by: on iOS we are using GetVisibleItem() everywhere (the iOS equivalent of
GetVisibleEntry). IIRC, when implementing WebFaviconDriver I dug into the
favicon code to know why GetActiveEntry was used and it is just because the code
pre-dated GetVisibleEntry/GetLastCommittedEntry. After testing we decided to use
GetVisibleItem().

   return entry ? entry->GetURL() : GURL();
 }
 
 base::string16 ContentFaviconDriver::GetActiveTitle() {
   content::NavigationEntry* entry =
-      web_contents()->GetController().GetActiveEntry();
+      web_contents()->GetController().GetLastCommittedEntry();
   return entry ? entry->GetTitle() : base::string16();
 }
 
 bool ContentFaviconDriver::GetActiveFaviconValidity() {
   return GetFaviconStatus().valid;
 }
 
 void ContentFaviconDriver::SetActiveFaviconValidity(bool valid) {
   GetFaviconStatus().valid = valid;
 }
 
 GURL ContentFaviconDriver::GetActiveFaviconURL() {
   return GetFaviconStatus().url;
 }
 
 void ContentFaviconDriver::SetActiveFaviconURL(const GURL& url) {
   GetFaviconStatus().url = url;
 }
 
 gfx::Image ContentFaviconDriver::GetActiveFaviconImage() {
   return GetFaviconStatus().image;
 }
 
 void ContentFaviconDriver::SetActiveFaviconImage(const gfx::Image& image) {
   GetFaviconStatus().image = image;
 }
 
 content::FaviconStatus& ContentFaviconDriver::GetFaviconStatus() {
-  DCHECK(web_contents()->GetController().GetActiveEntry());
-  return web_contents()->GetController().GetActiveEntry()->GetFavicon();
+  DCHECK(web_contents()->GetController().GetLastCommittedEntry());
+  return web_contents()->GetController().GetLastCommittedEntry()->GetFavicon();
 }
 
 ContentFaviconDriver::ContentFaviconDriver(
     content::WebContents* web_contents,
     FaviconService* favicon_service,
     history::HistoryService* history_service,
     bookmarks::BookmarkModel* bookmark_model)
     : content::WebContentsObserver(web_contents),
       FaviconDriverImpl(favicon_service, history_service, bookmark_model) {
 }
@@ skipping 39 matching lines @@
   // redownloaded.
   GURL url = details.entry->GetURL();
   if (url != bypass_cache_page_url_)
     bypass_cache_page_url_ = GURL();
 
   // Get the favicon, either from history or request it from the net.
   FetchFavicon(url);
 }
 
 }  // namespace favicon