Avoid cyclic dependency FaviconHandler<-->FaviconDriverImpl

chrome/browser/sessions/tab_loader.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 "chrome/browser/sessions/tab_loader.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/memory/memory_coordinator_client_registry.h"
@@ skipping 96 matching lines @@
   // restore is halted due to memory pressure. Also, when multiple tabs are
   // restored to a single window, the title may not appear, and the user will
   // have no way of finding out which tabs corresponds to which page if the icon
   // is a generic grey one.
   for (auto& restored_tab : tabs) {
     if (!restored_tab.is_active()) {
       tabs_to_load_.push_back(&restored_tab.contents()->GetController());
       favicon::ContentFaviconDriver* favicon_driver =
           favicon::ContentFaviconDriver::FromWebContents(
               restored_tab.contents());
+      // TODO(mastiz): This seems broken: one call cancels previous ones so this
+      // is unlikely to do any good.

[pkotwicz, 2017/02/22 02:37:53]

This seems like this should be a crbug not a comment in the code

[mastiz, 2017/02/23 21:55:53]

Ditto.

       // |favicon_driver| might be null when testing.
       if (favicon_driver)
         favicon_driver->FetchFavicon(favicon_driver->GetActiveURL());
     } else {
       tabs_loading_.insert(&restored_tab.contents()->GetController());
     }
     RegisterForNotifications(&restored_tab.contents()->GetController());
   }
 
   // When multiple profiles are using the same TabLoader, another profile might
@@ skipping 164 matching lines @@
     // memory pressure.
     stats_collector_->DeferTab(tab);
   }
   // By calling |LoadNextTab| explicitly, we make sure that the
   // |NOTIFICATION_SESSION_RESTORE_DONE| event gets sent.
   LoadNextTab();
 }
 
 // static
 TabLoader* TabLoader::shared_tab_loader_ = nullptr;