[go: up one dir, main page]

Reland "[upgrade_detector] Avoid the macOS Catalina file consent dialog."

This reverts commit 84ad51ea57c904571bcc6cb1372f67eb988100a2.

Reason for revert: This is not the right fix

Original change's description:
> Revert "[upgrade_detector] Avoid the macOS Catalina file consent dialog."
>
> This reverts commit df9d1d0c056736fced587d48487694d68e87f2ef.
>
> This revert switches from a "trivial" watch on macOS back to a
> non-recursive watch. This means that parent directories will also be
> monitored. It is this monitoring of parent dirs that may trigger the
> consent dialog.
>
> This revert is yet another attempt to address a crash in
> base::FilePathWatcherKQueue::OnKQueueReadable that appears to correlate
> with the landing of https://crrev.com/830353.
>
> TBR=grt@chromium.org
>
> Bug: 1147071,1156603
> Change-Id: I09f15fa1d668c18578e63cb011faa7047a07f102
> Fixed: 88
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2642159
> Auto-Submit: Greg Thompson <grt@chromium.org>
> Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Reviewed-by: Jan Wilken Dörrie <jdoerrie@chromium.org>
> Cr-Original-Original-Commit-Position: refs/heads/master@{#846666}
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2652896
> Reviewed-by: Greg Thompson <grt@chromium.org>
> Commit-Queue: Greg Thompson <grt@chromium.org>
> Cr-Commit-Position: refs/branch-heads/4324@{#2059}
> Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}

TBR=grt@chromium.org,jdoerrie@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1147071
Bug: 1156603
Change-Id: Ib251ad229f70bbaf94e0d156cb6fa3bcf2e0310d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2665346
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/branch-heads/4324@{#2085}
Cr-Branched-From: c73b5a651d37a6c4d0b8e3262cc4015a5579c6c8-refs/heads/master@{#827102}
diff --git a/chrome/browser/upgrade_detector/directory_monitor.cc b/chrome/browser/upgrade_detector/directory_monitor.cc
index d70c8ee..bf8af40 100644
--- a/chrome/browser/upgrade_detector/directory_monitor.cc
+++ b/chrome/browser/upgrade_detector/directory_monitor.cc
@@ -55,28 +55,40 @@
        base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN, base::MayBlock()});
   watcher_ = std::make_unique<base::FilePathWatcher>();
 
-  // Start the watcher on a background sequence, reporting all events back to
-  // this sequence. base::Unretained is safe because the watcher instance lives
-  // on the target sequence and will be destroyed there in a subsequent task.
+#if defined(OS_MAC)
+  // The normal Watch risks triggering a macOS Catalina+ consent dialog, so use
+  // a trivial watch here.
+  const base::FilePathWatcher::Type watch_type =
+      base::FilePathWatcher::Type::kTrivial;
+#else
+  const base::FilePathWatcher::Type watch_type =
+      base::FilePathWatcher::Type::kNonRecursive;
+#endif
+
+  // Start the watcher on a background sequence, reporting a failure to start to
+  // |on_change_callback| on the caller's sequence. The watcher is given a
+  // trampoline that will run |on_change_callback| on the caller's sequence.
+  // base::Unretained is safe because the watcher instance lives on the target
+  // sequence and will be destroyed there in a subsequent task.
   task_runner_->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(
           static_cast<bool (base::FilePathWatcher::*)(
-              const base::FilePath&, bool,
+              const base::FilePath&, base::FilePathWatcher::Type,
               const base::FilePathWatcher::Callback&)>(
               &base::FilePathWatcher::Watch),
-          base::Unretained(watcher_.get()), std::move(install_dir_),
-          /*recursive=*/false,
+          base::Unretained(watcher_.get()), std::move(install_dir_), watch_type,
           base::BindRepeating(
-              [](scoped_refptr<base::SequencedTaskRunner> main_sequence,
+              [](base::SequencedTaskRunner* main_sequence,
                  const Callback& on_change_callback, const base::FilePath&,
                  bool error) {
                 main_sequence->PostTask(
                     FROM_HERE, base::BindOnce(on_change_callback, error));
               },
-              base::SequencedTaskRunnerHandle::Get(), on_change_callback)),
+              base::RetainedRef(base::SequencedTaskRunnerHandle::Get()),
+              on_change_callback)),
       base::BindOnce(
-          [](Callback on_change_callback, bool start_result) {
+          [](const Callback& on_change_callback, bool start_result) {
             if (!start_result)
               on_change_callback.Run(/*error=*/true);
           },