Commit 769107aa authored by Natalie Chouinard's avatar Natalie Chouinard Committed by Commit Bot

Use unique_ptr for last_fetch_trigger_type_

Undefined behavior occurs (observed as a crash) when the Feed Internals
page is loaded and the last_fetch_trigger_type_ var is uninitialized,
so wrapping it here in a nullable pointer to avoid this.

Bug: 981513
Change-Id: I8637412423ab14d1a1329a414997d1f38887daaa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691046Reviewed-by: default avatarSky Malice <skym@chromium.org>
Commit-Queue: Natalie Chouinard <chouinard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675698}
parent 52befed6
...@@ -31,8 +31,10 @@ feed_internals::mojom::TimePtr ToMojoTime(base::Time time) { ...@@ -31,8 +31,10 @@ feed_internals::mojom::TimePtr ToMojoTime(base::Time time) {
: feed_internals::mojom::Time::New(time.ToJsTime()); : feed_internals::mojom::Time::New(time.ToJsTime());
} }
std::string TriggerTypeToString(feed::FeedSchedulerHost::TriggerType trigger) { std::string TriggerTypeToString(feed::FeedSchedulerHost::TriggerType* trigger) {
switch (trigger) { if (trigger == nullptr)
return "Not set";
switch (*trigger) {
case feed::FeedSchedulerHost::TriggerType::kNtpShown: case feed::FeedSchedulerHost::TriggerType::kNtpShown:
return "NTP Shown"; return "NTP Shown";
case feed::FeedSchedulerHost::TriggerType::kForegrounded: case feed::FeedSchedulerHost::TriggerType::kForegrounded:
......
...@@ -507,8 +507,8 @@ int FeedSchedulerHost::GetLastFetchStatusForDebugging() const { ...@@ -507,8 +507,8 @@ int FeedSchedulerHost::GetLastFetchStatusForDebugging() const {
return last_fetch_status_; return last_fetch_status_;
} }
TriggerType FeedSchedulerHost::GetLastFetchTriggerTypeForDebugging() const { TriggerType* FeedSchedulerHost::GetLastFetchTriggerTypeForDebugging() const {
return last_fetch_trigger_type_; return last_fetch_trigger_type_.get();
} }
void FeedSchedulerHost::OnEulaAccepted() { void FeedSchedulerHost::OnEulaAccepted() {
...@@ -602,7 +602,7 @@ FeedSchedulerHost::ShouldRefreshResult FeedSchedulerHost::ShouldRefresh( ...@@ -602,7 +602,7 @@ FeedSchedulerHost::ShouldRefreshResult FeedSchedulerHost::ShouldRefresh(
clock_->Now() + clock_->Now() +
base::TimeDelta::FromSeconds(kTimeoutDurationSeconds.Get()); base::TimeDelta::FromSeconds(kTimeoutDurationSeconds.Get());
last_fetch_trigger_type_ = trigger; last_fetch_trigger_type_ = std::make_unique<TriggerType>(trigger);
return kShouldRefresh; return kShouldRefresh;
} }
......
...@@ -146,7 +146,7 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer { ...@@ -146,7 +146,7 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer {
// Surface the TriggerType for the last ShouldRefresh check that resulted in // Surface the TriggerType for the last ShouldRefresh check that resulted in
// kShouldRefresh. Callers of ShouldRefresh are presumed to follow with the // kShouldRefresh. Callers of ShouldRefresh are presumed to follow with the
// actual refresh. // actual refresh.
TriggerType GetLastFetchTriggerTypeForDebugging() const; TriggerType* GetLastFetchTriggerTypeForDebugging() const;
private: private:
FRIEND_TEST_ALL_PREFIXES(FeedSchedulerHostTest, GetTriggerThreshold); FRIEND_TEST_ALL_PREFIXES(FeedSchedulerHostTest, GetTriggerThreshold);
...@@ -234,7 +234,7 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer { ...@@ -234,7 +234,7 @@ class FeedSchedulerHost : web_resource::EulaAcceptedNotifier::Observer {
int last_fetch_status_ = 0; int last_fetch_status_ = 0;
// Reason for last fetch for debugging. // Reason for last fetch for debugging.
TriggerType last_fetch_trigger_type_; std::unique_ptr<TriggerType> last_fetch_trigger_type_;
DISALLOW_COPY_AND_ASSIGN(FeedSchedulerHost); DISALLOW_COPY_AND_ASSIGN(FeedSchedulerHost);
}; };
......
...@@ -1040,19 +1040,19 @@ TEST_F(FeedSchedulerHostTest, GetLastFetchTriggerTypeForDebugging) { ...@@ -1040,19 +1040,19 @@ TEST_F(FeedSchedulerHostTest, GetLastFetchTriggerTypeForDebugging) {
scheduler()->OnForegrounded(); scheduler()->OnForegrounded();
EXPECT_EQ(FeedSchedulerHost::TriggerType::kForegrounded, EXPECT_EQ(FeedSchedulerHost::TriggerType::kForegrounded,
scheduler()->GetLastFetchTriggerTypeForDebugging()); *scheduler()->GetLastFetchTriggerTypeForDebugging());
scheduler()->OnArticlesCleared(/*suppress_refreshes*/ false); scheduler()->OnArticlesCleared(/*suppress_refreshes*/ false);
EXPECT_EQ(FeedSchedulerHost::TriggerType::kNtpShown, EXPECT_EQ(FeedSchedulerHost::TriggerType::kNtpShown,
scheduler()->GetLastFetchTriggerTypeForDebugging()); *scheduler()->GetLastFetchTriggerTypeForDebugging());
ClassifyAsActiveSuggestionsConsumer(); // Fixed timer at 48 hours. ClassifyAsActiveSuggestionsConsumer(); // Fixed timer at 48 hours.
test_clock()->Advance(TimeDelta::FromHours(49)); test_clock()->Advance(TimeDelta::FromHours(49));
scheduler()->OnFixedTimer(base::OnceClosure()); scheduler()->OnFixedTimer(base::OnceClosure());
EXPECT_EQ(FeedSchedulerHost::TriggerType::kFixedTimer, EXPECT_EQ(FeedSchedulerHost::TriggerType::kFixedTimer,
scheduler()->GetLastFetchTriggerTypeForDebugging()); *scheduler()->GetLastFetchTriggerTypeForDebugging());
} }
} // namespace feed } // namespace feed
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