DefaultBrowserWorker now fully supports opening the settings for Win10

chrome/browser/shell_integration_win.cc

 // Copyright (c) 2012 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/shell_integration.h"
 
 #include <windows.h>
 #include <shlwapi.h>
 #include <shobjidl.h>
 #include <propkey.h>  // Needs to come after shobjidl.h.
 #include <stddef.h>
 #include <stdint.h>
 
+#include <vector>
+
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/timer/timer.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_comptr.h"
 #include "base/win/scoped_propvariant.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
@@ skipping 174 matching lines @@
     case ShellUtil::NOT_DEFAULT:
       return DefaultWebClientState::NOT_DEFAULT;
     case ShellUtil::IS_DEFAULT:
       return DefaultWebClientState::IS_DEFAULT;
     default:
       DCHECK_EQ(ShellUtil::UNKNOWN_DEFAULT, default_state);
       return DefaultWebClientState::UNKNOWN_DEFAULT;
   }
 }
 
+bool IsSetAsDefaultUsingSystemSettings() {
+  return base::win::GetVersion() >= base::win::VERSION_WIN10;
+}
+
+// There is no way to make sure the user is done with the system settings, but a
+// signal that the interaction is finished is needed for UMA. A timer of 2
+// minutes is used as a substitute. The registry keys for the protocol
+// association with an app are also monitored to signal the end of the
+// interaction early when it is clear that the user made a choice (e.g. http
+// and https for default browser).
+//
+// This helper class manages both the timer and the registry watchers and makes
+// sure the callback for the end of the settings interaction is only run once.
+// This class also manages its own lifetime.
+class OpenSystemSettingsHelper {
+ public:
+  // Begin the monitoring and will call |on_finished_callback| when done.
+  // Takes in a null-terminated array of |protocols| whose registry keys must be
+  // watched.
+  static void Begin(const wchar_t* const protocols[],
+                    const base::Closure& on_finished_callback) {
+    new OpenSystemSettingsHelper(protocols, on_finished_callback);
+  }
+
+ private:
+  OpenSystemSettingsHelper(const wchar_t* const protocols[],
+                           const base::Closure& on_finished_callback)
+      : on_finished_callback_(on_finished_callback), weak_ptr_factory_(this) {
+    static const wchar_t kUrlAssociationFormat[] =
+        L"SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\"
+        L"%ls\\UserChoice";
+
+    int i = 0;

[grt (UTC plus 2), 2016/03/31 19:08:12]

you could get rid of |i| by scanning the array:
  for (const wchar_t* const* scan = &protocols[0]; *scan != nullptr; ++scan) {
    AddRegistryKeyWatcher(..., *scan, ...);
  }

[Patrick Monette, 2016/04/05 20:04:55]

Done.

+    while (protocols[i] != nullptr) {
+      AddRegistryKeyWatcher(
+          base::StringPrintf(kUrlAssociationFormat, protocols[i]).c_str());
+      i++;
+    }
+    // Only the watchers that were succesfully initialized are counted.
+    registry_watcher_count_ = registry_key_watchers_.size();
+
+    timer_.Start(FROM_HERE, base::TimeDelta::FromMinutes(2),
+                 base::Bind(&OpenSystemSettingsHelper::ConcludeInteraction,
+                            weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  // Called when a change is detected on one of the registry keys being watched.
+  // Note: All types of modification to the registry key will trigger this
+  //       function even if the value change is the only one that matters. This
+  //       is good enough for now.
+  void OnRegistryKeyChanged() {
+    DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+
+    // Make sure all the registry watchers have fired.
+    if (--registry_watcher_count_ == 0)
+      ConcludeInteraction();
+  }
+
+  // Ends the monitoring with the system settings. Will call
+  // |on_finished_callback_| and then dispose of this class instance to make
+  // sure the callback won't get called subsequently.
+  void ConcludeInteraction() {
+    DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+

[grt (UTC plus 2), 2016/03/31 19:08:12]

how about UMA here so we know if the interaction is concluded due to a timeout
vs due to keys being changed? might also be interesting to know how long it took
for the final OnRegistryKeyChanged to arrive. wdyt?

[Patrick Monette, 2016/04/05 20:04:55]

Done.

+    on_finished_callback_.Run();
+    delete this;
+  }
+
+  // Helper function to create a registry watcher for a given |key_path|. Do
+  // nothing on initialization failure.
+  void AddRegistryKeyWatcher(const wchar_t* key_path) {
+    auto reg_key = make_scoped_ptr(
+        new base::win::RegKey(HKEY_CURRENT_USER, key_path, KEY_NOTIFY));
+
+    if (reg_key->Valid() &&
+        reg_key->StartWatching(
+            base::Bind(&OpenSystemSettingsHelper::OnRegistryKeyChanged,
+                       weak_ptr_factory_.GetWeakPtr()))) {
+      registry_key_watchers_.push_back(std::move(reg_key));
+    }
+  }
+
+  // The function to call when the interaction with the system settings is
+  // finished.
+  base::Closure on_finished_callback_;
+
+  // The number of time the registry key watchers must fire.
+  int registry_watcher_count_ = 0;
+
+  // There can be multiple registry key watchers as some settings modify
+  // multiple protocol associations. e.g. Changing the default browser modifies
+  // the http and https associations.
+  std::vector<scoped_ptr<base::win::RegKey>> registry_key_watchers_;
+
+  base::OneShotTimer timer_;
+
+  // Weak ptrs are used to bind this class to the callbacks of the timer and the
+  // registry watcher. This makes it possible to self-delete after one of the
+  // callbacks is executed to cancel the remaining ones.
+  base::WeakPtrFactory<OpenSystemSettingsHelper> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(OpenSystemSettingsHelper);
+};
+
 }  // namespace
 
 bool SetAsDefaultBrowser() {
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
     LOG(ERROR) << "Error getting app exe path";
     return false;
   }
 
   // From UI currently we only allow setting default browser for current user.
@@ skipping 18 matching lines @@
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   if (!ShellUtil::ShowMakeChromeDefaultSystemUI(dist, chrome_exe)) {
     LOG(ERROR) << "Failed to launch the set-default-browser Windows UI.";
     return false;
   }
 
   VLOG(1) << "Set-default-browser Windows UI completed.";
   return true;
 }
 
+void SetAsDefaultBrowserUsingSystemSettings(
+    const base::Closure& on_finished_callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+
+  // The helper manages its own lifetime.
+  static const wchar_t* const kProtocols[] = {L"http", L"https", nullptr};
+  OpenSystemSettingsHelper::Begin(kProtocols, on_finished_callback);
+
+  base::FilePath chrome_exe;
+  if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
+    NOTREACHED() << "Error getting app exe path";
+    return;

[grt (UTC plus 2), 2016/03/31 19:08:12]

wdyt of running the callback before returning here, and only starting the helper
down below? there's no need to wait two minutes before running the callback if
this happens. then again, i don't think this could ever fail, so maybe the
return value should just be ignored?

[Patrick Monette, 2016/04/05 20:04:55]

Done. The logging is consistent with the other SetASDefault() functions.

+  }
+
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  ShellUtil::ShowMakeChromeDefaultSystemUI(dist, chrome_exe);
+}
+
 bool SetAsDefaultProtocolClient(const std::string& protocol) {
   if (protocol.empty())
     return false;
 
   base::FilePath chrome_exe;
   if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
     LOG(ERROR) << "Error getting app exe path";
     return false;
   }
 
@@ skipping 29 matching lines @@
   return true;
 }
 
 DefaultWebClientSetPermission CanSetAsDefaultBrowser() {
   BrowserDistribution* distribution = BrowserDistribution::GetDistribution();
   if (distribution->GetDefaultBrowserControlPolicy() !=
           BrowserDistribution::DEFAULT_BROWSER_FULL_CONTROL)
     return SET_DEFAULT_NOT_ALLOWED;
   if (ShellUtil::CanMakeChromeDefaultUnattended())
     return SET_DEFAULT_UNATTENDED;
+  if (IsSetAsDefaultUsingSystemSettings())
+    return SET_DEFAULT_OPEN_SETTINGS;
   return SET_DEFAULT_INTERACTIVE;
 }
 
 bool IsElevationNeededForSettingDefaultProtocolClient() {
   return base::win::GetVersion() < base::win::VERSION_WIN8;
 }
 
 base::string16 GetApplicationNameForProtocol(const GURL& url) {
   // Windows 8 or above requires a new protocol association query.
   if (base::win::GetVersion() >= base::win::VERSION_WIN8)
@@ skipping 220 matching lines @@
 
     shortcut = programs_folder.Append(shortcut_name);
     if (base::PathExists(shortcut))
       return shortcut;
   }
 
   return base::FilePath();
 }
 
 }  // namespace shell_integration