Skip to content

Commit

Permalink
[DownloadWarningHaTS] Add helper class for delayed "ignore" surveys
Browse files Browse the repository at this point in the history
This adds a helper class that manages the delayed surveys that are shown
after a download warning is ignored for some amount of time while the
user is otherwise still actively using Chrome.

Managing the lifetimes of the callbacks is the bulk of the work it does.
It observes each DownloadItem for which it schedules a task, and if that
item is deleted or becomes ineligible for the HaTS surveys, it cancels
the task. The delayed survey tasks can also be canceled explicitly by
the client, e.g. when the user interacts with the download.

This is required for showing surveys after the user ignores a download
bubble warning for 5 minutes, implemented in the next CL.

Bug: 327457604
Change-Id: Iab1cbba431fd7408c645060853467a0296dcd5de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5526430
Reviewed-by: Daniel Rubery <[email protected]>
Commit-Queue: Lily Chen <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1300221}
  • Loading branch information
chlily1 authored and Chromium LUCI CQ committed May 13, 2024
1 parent d9e3b09 commit b879590
Show file tree
Hide file tree
Showing 3 changed files with 408 additions and 34 deletions.
129 changes: 125 additions & 4 deletions chrome/browser/download/download_warning_desktop_hats_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,20 @@

#include "chrome/browser/download/download_warning_desktop_hats_utils.h"

#include <cstdint>
#include <iterator>
#include <string>
#include <vector>

#include "base/containers/contains.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/ranges/algorithm.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/sequenced_task_runner.h"
#include "base/time/time.h"
#include "base/version_info/channel.h"
#include "build/build_config.h"
Expand Down Expand Up @@ -339,6 +343,121 @@ DownloadWarningHatsProductSpecificData::GetStringDataFields(
return fields;
}

DelayedDownloadWarningHatsLauncher::Task::Task(
DelayedDownloadWarningHatsLauncher& hats_launcher,
download::DownloadItem* download,
base::OnceClosure task,
base::TimeDelta delay)
: observation_(&hats_launcher), task_(std::move(task)) {
observation_.Observe(download);
base::SequencedTaskRunner::GetCurrentDefault()->PostDelayedTask(
FROM_HERE, base::BindOnce(&Task::RunTask, weak_factory_.GetWeakPtr()),
delay);
}

DelayedDownloadWarningHatsLauncher::Task::~Task() = default;

// It is expected that the caller will delete this after this executes.
void DelayedDownloadWarningHatsLauncher::Task::RunTask() {
CHECK(task_);
std::move(task_).Run();
}

DelayedDownloadWarningHatsLauncher::DelayedDownloadWarningHatsLauncher(
Profile* profile,
base::TimeDelta delay,
PsdCompleter psd_completer)
: profile_(profile),
delay_(delay),
psd_completer_(std::move(psd_completer)) {}

DelayedDownloadWarningHatsLauncher::~DelayedDownloadWarningHatsLauncher() =
default;

void DelayedDownloadWarningHatsLauncher::OnDownloadUpdated(
download::DownloadItem* download) {
// If the formerly-eligible download is no longer eligible, cancel the survey.
if (!CanShowDownloadWarningHatsSurvey(download)) {
RemoveTaskIfAny(download);
}
}

void DelayedDownloadWarningHatsLauncher::OnDownloadDestroyed(
download::DownloadItem* download) {
RemoveTaskIfAny(download);
}

void DelayedDownloadWarningHatsLauncher::RecordBrowserActivity() {
last_activity_ = base::Time::Now();
}

bool DelayedDownloadWarningHatsLauncher::TryScheduleTask(
DownloadWarningHatsType survey_type,
download::DownloadItem* download) {
CHECK(download);
TaskKey key = reinterpret_cast<TaskKey>(download);
if (base::Contains(tasks_, key)) {
return false;
}

if (!CanShowDownloadWarningHatsSurvey(download)) {
return false;
}

auto [_, inserted] = tasks_.try_emplace(
key, *this, download,
// Unretained is safe because `this` outlives the Task object.
// `download` will be valid for as long as the Task lives, because the
// Observer mechanism will delete the Task if `download` goes away.
base::BindOnce(&DelayedDownloadWarningHatsLauncher::MaybeLaunchSurveyNow,
base::Unretained(this), survey_type, download),
delay_);
return inserted;
}

void DelayedDownloadWarningHatsLauncher::RemoveTaskIfAny(
download::DownloadItem* download) {
TaskKey key = reinterpret_cast<TaskKey>(download);
tasks_.erase(key);
}

void DelayedDownloadWarningHatsLauncher::MaybeLaunchSurveyNow(
DownloadWarningHatsType survey_type,
download::DownloadItem* download) {
if (!CanShowDownloadWarningHatsSurvey(download) || !WasUserActive()) {
RemoveTaskIfAny(download);
return;
}

auto psd =
DownloadWarningHatsProductSpecificData::Create(survey_type, download);
if (psd_completer_) {
psd_completer_.Run(psd);
}

MaybeLaunchDownloadWarningHatsSurvey(profile_, psd,
MakeSurveyDoneCallback(download),
MakeSurveyDoneCallback(download));
}

base::OnceClosure DelayedDownloadWarningHatsLauncher::MakeSurveyDoneCallback(
download::DownloadItem* download) {
// This is needed to clean up the Task object after the survey runs. It must
// be bound to a WeakPtr because nothing guarantees that this will be alive
// when the survey task finishes (it generally takes a few seconds to actually
// show the survey, and obviously takes much longer for the user to work
// through the survey). If this callback runs, then `this` must still be
// alive, which means the DownloadItem::Observer mechanism is maintaining the
// invariant that any download with an entry in `tasks_` must be alive.
// Therefore, `download` will not be dereferenced while dangling.
return base::BindOnce(&DelayedDownloadWarningHatsLauncher::RemoveTaskIfAny,
weak_factory_.GetWeakPtr(), download);
}

bool DelayedDownloadWarningHatsLauncher::WasUserActive() const {
return base::Time::Now() - last_activity_ <= delay_;
}

bool CanShowDownloadWarningHatsSurvey(download::DownloadItem* download) {
CHECK(download);
return download->IsDangerous() && !download->IsDone();
Expand Down Expand Up @@ -384,7 +503,9 @@ std::optional<std::string> MaybeGetDownloadWarningHatsTrigger(

void MaybeLaunchDownloadWarningHatsSurvey(
Profile* profile,
const DownloadWarningHatsProductSpecificData& psd) {
const DownloadWarningHatsProductSpecificData& psd,
base::OnceClosure success_callback,
base::OnceClosure failure_callback) {
std::optional<std::string> trigger =
MaybeGetDownloadWarningHatsTrigger(psd.survey_type());
if (!trigger) {
Expand All @@ -394,8 +515,8 @@ void MaybeLaunchDownloadWarningHatsSurvey(
HatsService* hats_service =
HatsServiceFactory::GetForProfile(profile, /*create_if_necessary=*/true);
if (hats_service) {
hats_service->LaunchSurvey(*trigger, /*success_callback=*/base::DoNothing(),
/*failure_callback=*/base::DoNothing(),
psd.bits_data(), psd.string_data());
hats_service->LaunchSurvey(*trigger, std::move(success_callback),
std::move(failure_callback), psd.bits_data(),
psd.string_data());
}
}
127 changes: 122 additions & 5 deletions chrome/browser/download/download_warning_desktop_hats_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
#ifndef CHROME_BROWSER_DOWNLOAD_DOWNLOAD_WARNING_DESKTOP_HATS_UTILS_H_
#define CHROME_BROWSER_DOWNLOAD_DOWNLOAD_WARNING_DESKTOP_HATS_UTILS_H_

#include <map>
#include <optional>
#include <string>
#include <vector>

#include "base/functional/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "chrome/browser/ui/hats/hats_service.h"

namespace download {
class DownloadItem;
}
#include "components/download/public/common/download_item.h"

// Type of survey (corresponding to a trigger condition) that should be shown.
// Do not renumber.
Expand Down Expand Up @@ -154,6 +155,120 @@ class DownloadWarningHatsProductSpecificData {
SurveyStringData string_data_;
};

// A class that manages delayed download warning HaTS survey tasks. It can be
// given a DownloadItem to launch a survey for in the future after some delay,
// and these tasks can be canceled explicitly or automatically (in case of
// the DownloadItem getting destroyed or becoming ineligible for a HaTS survey).
// It also records the last time the user interacted with the browser, and the
// survey is withheld if the user was (presumably) idle for the entire period of
// the delay. (Client should inform this object of browser activity.)
// Note: Currently this is only used for download bubble ignore triggers.
class DelayedDownloadWarningHatsLauncher
: public download::DownloadItem::Observer {
public:
// A callback that allows the completion of the PSD (addition of post-Create()
// fields).
using PsdCompleter =
base::RepeatingCallback<void(DownloadWarningHatsProductSpecificData&)>;

// Bundles the objects used to control the task and its lifetime. Can only be
// used once per instance. To cancel, delete this object. The `download`
// and `hats_launcher` must outlive this.
class Task {
public:
// Creates and schedules the task.
Task(DelayedDownloadWarningHatsLauncher& hats_launcher,
download::DownloadItem* download,
base::OnceClosure task,
base::TimeDelta delay);

Task(const Task&) = delete;
Task& operator=(const Task&) = delete;

~Task();

private:
void RunTask();

// Controls the observation of the download by the parent object.
base::ScopedObservation<download::DownloadItem,
DelayedDownloadWarningHatsLauncher>
observation_;
// Task to show the survey.
base::OnceClosure task_;
// Used to cancel the scheduled task.
base::WeakPtrFactory<Task> weak_factory_{this};
};

// `profile` is the profile for which the HaTS surveys should be shown.
// `delay` is the delay that applies to all surveys launched by this object.
// `psd_completer` will be called with the product-specific data right before
// attempting to launch each survey.
DelayedDownloadWarningHatsLauncher(
Profile* profile,
base::TimeDelta delay,
PsdCompleter psd_completer = base::DoNothing());

DelayedDownloadWarningHatsLauncher(
const DelayedDownloadWarningHatsLauncher&) = delete;
DelayedDownloadWarningHatsLauncher& operator=(
const DelayedDownloadWarningHatsLauncher&) = delete;

~DelayedDownloadWarningHatsLauncher() override;

// download::DownloadItem::Observer:
// This object is an observer of every download with an entry in `tasks_`.
void OnDownloadUpdated(download::DownloadItem* download) override;
void OnDownloadDestroyed(download::DownloadItem* download) override;

// Updates the last_activity_ time.
void RecordBrowserActivity();

// Schedules a survey to be shown after the delay, if the user has been active
// in the meantime. Does nothing if a task already exists for the download.
// Does nothing if the download is not eligible when scheduling. (The
// scheduled task will also fizzle if the download is not eligible upon
// execution.) Returns whether task was scheduled.
bool TryScheduleTask(DownloadWarningHatsType survey_type,
download::DownloadItem* download);

// Cancels and removes the task for `download` from the map. Is a no-op if the
// download is not in the map.
void RemoveTaskIfAny(download::DownloadItem* download);

private:
// Address of a DownloadItem, derived from a DownloadItem*, but it is not to
// be dereferenced.
using TaskKey = std::uintptr_t;
using TasksMap = std::map<TaskKey, Task>;

// Launches the actual survey, if all preconditions are met.
void MaybeLaunchSurveyNow(DownloadWarningHatsType survey_type,
download::DownloadItem* download);

// Returns a callback that is called to clean up after the survey succeeds or
// fails.
base::OnceClosure MakeSurveyDoneCallback(download::DownloadItem* download);

// Whether the user was active in the browser during the delay period.
bool WasUserActive() const;

// Profile to show the surveys for. Must outlive this.
const raw_ptr<Profile> profile_;
// How long to wait before launching the survey.
const base::TimeDelta delay_;
// Time of the most recent user interaction with the browser.
base::Time last_activity_;
// Maps DownloadItem addresses to their corresponding pending tasks.
TasksMap tasks_;
// Callback that is run to stamp the PSD with any additional fields right
// before attempting to launch the survey.
PsdCompleter psd_completer_;
// Needed because the cleanup callback produced by MakeSurveyDoneCallback
// may outlive this.
base::WeakPtrFactory<DelayedDownloadWarningHatsLauncher> weak_factory_{this};
};

// Returns if the download item is dangerous and not-done.
bool CanShowDownloadWarningHatsSurvey(download::DownloadItem* download);

Expand All @@ -176,6 +291,8 @@ std::optional<std::string> MaybeGetDownloadWarningHatsTrigger(
// launch the survey.
void MaybeLaunchDownloadWarningHatsSurvey(
Profile* profile,
const DownloadWarningHatsProductSpecificData& psd);
const DownloadWarningHatsProductSpecificData& psd,
base::OnceClosure success_callback = base::DoNothing(),
base::OnceClosure failure_callback = base::DoNothing());

#endif // CHROME_BROWSER_DOWNLOAD_DOWNLOAD_WARNING_DESKTOP_HATS_UTILS_H_
Loading

0 comments on commit b879590

Please sign in to comment.