Skip to content

Commit

Permalink
favicons: don't create FaviconService if HistoryService is null
Browse files Browse the repository at this point in the history
My suspicion is history initialization is failing. I'm adding some
logging to verify that.

No doubt if faviconservice creation returns null there will be other
places assuming non-null that need to be fixed.

BUG=1024959, 1052127
TEST=none

Change-Id: I425e0b32d8543f46c1db7f18b232bbd65c898b94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2057093
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741561}
  • Loading branch information
Scott Violet authored and Commit Bot committed Feb 14, 2020
1 parent 3d70699 commit 955059f
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
14 changes: 10 additions & 4 deletions chrome/browser/favicon/favicon_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#include <memory>

#include "base/bind.h"
#include "base/memory/ptr_util.h"
#include "base/memory/singleton.h"
#include "chrome/browser/favicon/chrome_favicon_client.h"
#include "chrome/browser/history/history_service_factory.h"
Expand All @@ -22,10 +21,17 @@ namespace {
std::unique_ptr<KeyedService> BuildFaviconService(
content::BrowserContext* context) {
Profile* profile = Profile::FromBrowserContext(context);
return std::make_unique<favicon::FaviconServiceImpl>(
base::WrapUnique(new ChromeFaviconClient(profile)),
history::HistoryService* history_service =
HistoryServiceFactory::GetForProfile(profile,
ServiceAccessType::EXPLICIT_ACCESS));
ServiceAccessType::EXPLICIT_ACCESS);
// |history_service| may be null, most likely because initialization failed.
if (!history_service) {
// This is rare enough that it's worth logging.
LOG(WARNING) << "FaviconService not created as HistoryService is null";
return nullptr;
}
return std::make_unique<favicon::FaviconServiceImpl>(
std::make_unique<ChromeFaviconClient>(profile), history_service);
}

} // namespace
Expand Down
6 changes: 5 additions & 1 deletion components/history/core/browser/history_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,12 @@ bool HistoryService::Init(
base::BindRepeating(base::IgnoreResult(&HistoryService::ScheduleDBTask),
base::Unretained(this)));

if (visit_delegate_ && !visit_delegate_->Init(this))
if (visit_delegate_ && !visit_delegate_->Init(this)) {
// This is rare enough that it's worth logging.
LOG(WARNING) << "HistoryService::Init() failed by way of "
"VisitDelegate::Init failing";
return false;
}

if (history_client_)
history_client_->OnHistoryServiceCreated(this);
Expand Down

0 comments on commit 955059f

Please sign in to comment.