[HBD] Gate the advertising of Flash on Site Engagement.

chrome/browser/engagement/site_engagement_score.cc

 // Copyright 2016 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/engagement/site_engagement_score.h"
 
 #include <cmath>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/engagement/site_engagement_metrics.h"
-#include "chrome/browser/profiles/profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/variations/variations_associated_data.h"
 
 namespace {
 
 // Delta within which to consider scores equal.
 const double kScoreDelta = 0.001;
 
 // Delta within which to consider internal time values equal. Internal time
 // values are in microseconds, so this delta comes out at one second.
 const double kTimeDelta = 1000000;
 
 // Number of days after the last launch of an origin from an installed shortcut
 // for which WEB_APP_INSTALLED_POINTS will be added to the engagement score.
 const int kMaxDaysSinceShortcutLaunch = 10;
 
 bool DoublesConsideredDifferent(double value1, double value2, double delta) {
   double abs_difference = fabs(value1 - value2);
   return abs_difference > delta;
 }
 
-std::unique_ptr<base::DictionaryValue> GetScoreDictForOrigin(
-    Profile* profile,
+std::unique_ptr<base::DictionaryValue> GetScoreDictForSettings(
+    const HostContentSettingsMap* settings,
     const GURL& origin_url) {
-  HostContentSettingsMap* settings =
-      HostContentSettingsMapFactory::GetForProfile(profile);
-  HostContentSettingsMap* fallback_settings =
-      profile->IsOffTheRecord() ? HostContentSettingsMapFactory::GetForProfile(
-                                      profile->GetOriginalProfile())
-                                : nullptr;
-
   if (!settings)
-    return std::unique_ptr<base::DictionaryValue>();
+    return base::MakeUnique<base::DictionaryValue>();
 
   std::unique_ptr<base::Value> value = settings->GetWebsiteSetting(
       origin_url, origin_url, CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
       std::string(), NULL);
-  if (!value.get() && fallback_settings) {
-    value = fallback_settings->GetWebsiteSetting(
-        origin_url, origin_url, CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
-        std::string(), NULL);
-  }
 
-  if (!value.get())
-    return base::WrapUnique(new base::DictionaryValue());
-
-  if (!value->IsType(base::Value::TYPE_DICTIONARY))
-    return base::WrapUnique(new base::DictionaryValue());
+  if (!value.get() || !value->IsType(base::Value::TYPE_DICTIONARY))
+    return base::MakeUnique<base::DictionaryValue>();
 
   return base::WrapUnique(static_cast<base::DictionaryValue*>(value.release()));
 }
 
 }  // namespace
 
 const double SiteEngagementScore::kMaxPoints = 100;
 
 const char* SiteEngagementScore::kRawScoreKey = "rawScore";
 const char* SiteEngagementScore::kPointsAddedTodayKey = "pointsAddedToday";
@@ skipping 110 matching lines @@
       return;
     }
   }
 
   // Once we're sure everything is valid, assign the variation to the param
   // values array.
   for (int i = 0; i < MAX_VARIATION; ++i)
     SiteEngagementScore::GetParamValues()[i].second = param_vals[i];
 }
 
-SiteEngagementScore::SiteEngagementScore(base::Clock* clock,
-                                         const GURL& origin,
-                                         Profile* profile)
-    : SiteEngagementScore(clock, GetScoreDictForOrigin(profile, origin)) {
-  origin_ = origin;
-  profile_ = profile;
+SiteEngagementScore::SiteEngagementScore(
+    base::Clock* clock,
+    const GURL& origin,
+    HostContentSettingsMap* settings)
+    : SiteEngagementScore(
+          clock,
+          origin,
+          GetScoreDictForSettings(settings, origin)) {
+  settings_map_ = settings;
 }
 
 SiteEngagementScore::SiteEngagementScore(SiteEngagementScore&& other) = default;
 
 SiteEngagementScore::~SiteEngagementScore() {}
 
 SiteEngagementScore& SiteEngagementScore::operator=(
     SiteEngagementScore&& other) = default;
 
 void SiteEngagementScore::AddPoints(double points) {
@@ skipping 31 matching lines @@
   raw_score_ += to_add;
 
   last_engagement_time_ = now;
 }
 
 double SiteEngagementScore::GetScore() const {
   return std::min(DecayedScore() + BonusScore(), kMaxPoints);
 }
 
 void SiteEngagementScore::Commit() {
-  if (!UpdateScoreDict(score_dict_.get()))
+  if (!UpdateScoreDict(score_dict_.get()) || !settings_map_)

[calamity, 2016/09/13 04:14:37]

Is the settings_map_ ever null? We may want to DCHECK it instead.

[dominickn, 2016/09/13 04:42:14]

It could be nullptr in tests (where it uses the private constructor that doesn't
initialize settings_map_). The constructor also doesn't assume that it gets a
non-nullptr (e.g the nullptr check in GetScoreDictForSettings).

Unless we mandate that a non-null pointer is passed in during tests as well, I
don't think that it makes sense to DCHECK here, since you could legitimately
create a SiteEngagementScore using a nullptr. Otherwise, we should go the whole
hog and enforce a non-nullptr settings_map_ - including in the private
constructor. That makes SiteEngagementScore tests more complicated (that class
would probably need to be converted to subclass ChromeRenderViewHostTestHarness
so that there's a profile() etc. around).

[calamity, 2016/09/13 06:33:34]

To me, the condition the DCHECK here enforces is 'You shouldn't be calling
Commit() without a settings map'. The existing tests don't do this, and future
tests never should. I think it would be a useful tripwire that we can remove
later if it proves to be inconvenient.

[dominickn, 2016/09/14 01:06:55]

Done.

     return;
 
-  HostContentSettingsMapFactory::GetForProfile(profile_)
-      ->SetWebsiteSettingDefaultScope(origin_, GURL(),
-                                      CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT,
-                                      std::string(), std::move(score_dict_));
+  settings_map_->SetWebsiteSettingDefaultScope(
+      origin_, GURL(), CONTENT_SETTINGS_TYPE_SITE_ENGAGEMENT, std::string(),
+      std::move(score_dict_));
 }
 
 bool SiteEngagementScore::MaxPointsPerDayAdded() const {
   if (!last_engagement_time_.is_null() &&
       clock_->Now().LocalMidnight() != last_engagement_time_.LocalMidnight()) {
     return false;
   }
 
   return points_added_today_ == GetMaxPointsPerDay();
 }
@@ skipping 38 matching lines @@
   score_dict->SetDouble(kLastEngagementTimeKey,
                         last_engagement_time_.ToInternalValue());
   score_dict->SetDouble(kLastShortcutLaunchTimeKey,
                         last_shortcut_launch_time_.ToInternalValue());
 
   return true;
 }
 
 SiteEngagementScore::SiteEngagementScore(
     base::Clock* clock,
+    const GURL& origin,
     std::unique_ptr<base::DictionaryValue> score_dict)
     : clock_(clock),
       raw_score_(0),
       points_added_today_(0),
       last_engagement_time_(),
       last_shortcut_launch_time_(),
-      score_dict_(score_dict.release()) {
+      score_dict_(score_dict.release()),
+      origin_(origin) {
   if (!score_dict_)
     return;
 
   score_dict_->GetDouble(kRawScoreKey, &raw_score_);
   score_dict_->GetDouble(kPointsAddedTodayKey, &points_added_today_);
 
   double internal_time;
   if (score_dict_->GetDouble(kLastEngagementTimeKey, &internal_time))
     last_engagement_time_ = base::Time::FromInternalValue(internal_time);
   if (score_dict_->GetDouble(kLastShortcutLaunchTimeKey, &internal_time))
@@ skipping 38 matching lines @@
   GetParamValues()[HIGH_ENGAGEMENT_BOUNDARY].second = 50;
   GetParamValues()[MAX_DECAYS_PER_SCORE].second = 1;
   GetParamValues()[LAST_ENGAGEMENT_GRACE_PERIOD_IN_HOURS].second = 72;
 
   // This is set to values that avoid interference with tests and are set when
   // testing these features.
   GetParamValues()[FIRST_DAILY_ENGAGEMENT].second = 0;
   GetParamValues()[DECAY_PROPORTION].second = 1;
   GetParamValues()[SCORE_CLEANUP_THRESHOLD].second = 0;
 }