Commit b15c2fb0 authored by Alexandr Ilin's avatar Alexandr Ilin Committed by Commit Bot

predictors: Fix WebUI crash due to the lazy initialization.

Navigation to chrome://predictors cause a crash if it's the first
navigation after profile creation. The reason is that ResourcePrefetchPredictor
is initialized only after the first commited navigation excluding a navigation
to NTP. On the other hand, WebUI predictors handler doesn't check that the
predictor was initialized before accessing the field that is nullptr in
uninitialized state.

This CL turns on the initialization of ResourcePrefetchPredictor after the
navigation to NTP and adds a check for a uninitialized state in WebUI handler.

Bug: 715525
Change-Id: I9382e892ce713d2b0beb406be344df64e2d5dd3d
Reviewed-on: https://chromium-review.googlesource.com/575989
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: default avatarBenoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487465}
parent 5e7e2953
...@@ -342,6 +342,12 @@ void LoadingDataCollector::RecordMainFrameLoadComplete( ...@@ -342,6 +342,12 @@ void LoadingDataCollector::RecordMainFrameLoadComplete(
if (navigation_id.main_frame_url.is_empty()) if (navigation_id.main_frame_url.is_empty())
return; return;
// Initialize |predictor_| no matter whether the |navigation_id| is present in
// |inflight_navigations_|. This is the case for NTP and about:blank pages,
// for example.
if (predictor_)
predictor_->StartInitialization();
NavigationMap::iterator nav_it = inflight_navigations_.find(navigation_id); NavigationMap::iterator nav_it = inflight_navigations_.find(navigation_id);
if (nav_it == inflight_navigations_.end()) if (nav_it == inflight_navigations_.end())
return; return;
...@@ -359,9 +365,8 @@ void LoadingDataCollector::RecordMainFrameLoadComplete( ...@@ -359,9 +365,8 @@ void LoadingDataCollector::RecordMainFrameLoadComplete(
if (stats_collector_) if (stats_collector_)
stats_collector_->RecordPageRequestSummary(*summary); stats_collector_->RecordPageRequestSummary(*summary);
if (predictor_) { if (predictor_)
predictor_->RecordPageRequestSummary(std::move(summary)); predictor_->RecordPageRequestSummary(std::move(summary));
}
} }
void LoadingDataCollector::RecordFirstContentfulPaint( void LoadingDataCollector::RecordFirstContentfulPaint(
......
...@@ -56,8 +56,6 @@ void LoadingPredictorTest::SetUp() { ...@@ -56,8 +56,6 @@ void LoadingPredictorTest::SetUp() {
auto mock = base::MakeUnique<StrictMock<MockResourcePrefetchPredictor>>( auto mock = base::MakeUnique<StrictMock<MockResourcePrefetchPredictor>>(
config, profile_.get()); config, profile_.get());
EXPECT_CALL(*mock, StartInitialization());
EXPECT_CALL(*mock, Shutdown());
EXPECT_CALL(*mock, GetPrefetchData(GURL(kUrl), _)) EXPECT_CALL(*mock, GetPrefetchData(GURL(kUrl), _))
.WillRepeatedly(Return(true)); .WillRepeatedly(Return(true));
EXPECT_CALL(*mock, GetPrefetchData(GURL(kUrl2), _)) EXPECT_CALL(*mock, GetPrefetchData(GURL(kUrl2), _))
......
...@@ -36,11 +36,6 @@ class MockResourcePrefetchPredictor : public ResourcePrefetchPredictor { ...@@ -36,11 +36,6 @@ class MockResourcePrefetchPredictor : public ResourcePrefetchPredictor {
MOCK_CONST_METHOD2(GetPrefetchData, MOCK_CONST_METHOD2(GetPrefetchData,
bool(const GURL&, ResourcePrefetchPredictor::Prediction*)); bool(const GURL&, ResourcePrefetchPredictor::Prediction*));
MOCK_METHOD0(StartInitialization, void());
MOCK_METHOD0(Shutdown, void());
MOCK_METHOD2(StartPrefetching,
void(const GURL&, const ResourcePrefetchPredictor::Prediction&));
MOCK_METHOD1(StopPrefeching, void(const GURL&));
MOCK_METHOD1(RecordPageRequestSummaryProxy, void(PageRequestSummary*)); MOCK_METHOD1(RecordPageRequestSummaryProxy, void(PageRequestSummary*));
}; };
......
...@@ -99,25 +99,31 @@ void PredictorsHandler::RequestResourcePrefetchPredictorDb( ...@@ -99,25 +99,31 @@ void PredictorsHandler::RequestResourcePrefetchPredictorDb(
if (enabled) { if (enabled) {
auto* resource_prefetch_predictor = auto* resource_prefetch_predictor =
loading_predictor_->resource_prefetch_predictor(); loading_predictor_->resource_prefetch_predictor();
// URL table cache. const bool initialized =
auto db = base::MakeUnique<base::ListValue>(); resource_prefetch_predictor->initialization_state_ ==
AddPrefetchDataMapToListValue( ResourcePrefetchPredictor::INITIALIZED;
*resource_prefetch_predictor->url_resource_data_->data_cache_,
db.get()); if (initialized) {
dict.Set("url_db", std::move(db)); // URL table cache.
auto db = base::MakeUnique<base::ListValue>();
// Host table cache. AddPrefetchDataMapToListValue(
db = base::MakeUnique<base::ListValue>(); *resource_prefetch_predictor->url_resource_data_->data_cache_,
AddPrefetchDataMapToListValue( db.get());
*resource_prefetch_predictor->host_resource_data_->data_cache_, dict.Set("url_db", std::move(db));
db.get());
dict.Set("host_db", std::move(db)); // Host table cache.
db = base::MakeUnique<base::ListValue>();
// Origin table cache. AddPrefetchDataMapToListValue(
db = base::MakeUnique<base::ListValue>(); *resource_prefetch_predictor->host_resource_data_->data_cache_,
AddOriginDataMapToListValue( db.get());
*resource_prefetch_predictor->origin_data_->data_cache_, db.get()); dict.Set("host_db", std::move(db));
dict.Set("origin_db", std::move(db));
// Origin table cache.
db = base::MakeUnique<base::ListValue>();
AddOriginDataMapToListValue(
*resource_prefetch_predictor->origin_data_->data_cache_, db.get());
dict.Set("origin_db", std::move(db));
}
} }
web_ui()->CallJavascriptFunctionUnsafe("updateResourcePrefetchPredictorDb", web_ui()->CallJavascriptFunctionUnsafe("updateResourcePrefetchPredictorDb",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment