Setting chrome as the default browser is now fixed on Windows 10

chrome/browser/shell_integration.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 "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/timer/timer.h"
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_thread.h"
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/chromeos_switches.h"
 #endif
 
 #if !defined(OS_WIN)
 #include "chrome/common/channel_info.h"
 #include "chrome/grit/chromium_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #endif
 
 using content::BrowserThread;
 
 // static
 ShellIntegration::DefaultWebClientSetPermission
     ShellIntegration::CanSetAsDefaultProtocolClient() {
   // Allowed as long as the browser can become the operating system default
   // browser.
-  return CanSetAsDefaultBrowser();
+  DefaultWebClientSetPermission permission = CanSetAsDefaultBrowser();
+
+  // Set as default asynchronous is only supported for default web browser.
+  return (permission == SET_DEFAULT_ASYNCHRONOUS) ? SET_DEFAULT_INTERACTIVE
+                                                  : permission;
 }
 
 static const struct ShellIntegration::AppModeInfo* gAppModeInfo = NULL;
 
 // static
 void ShellIntegration::SetAppModeInfo(const struct AppModeInfo* info) {
   gAppModeInfo = info;
 }
 
 // static
@@ skipping 69 matching lines @@
   if (chrome::GetChannel() == version_info::Channel::CANARY)
     return l10n_util::GetStringUTF16(IDS_APP_SHORTCUTS_SUBDIR_NAME_CANARY);
   return l10n_util::GetStringUTF16(IDS_APP_SHORTCUTS_SUBDIR_NAME);
 }
 
 // static
 bool ShellIntegration::SetAsDefaultBrowserInteractive() {
   return false;
 }
 
+#if !defined(OS_WIN)
+// static
+bool ShellIntegration::IsSetAsDefaultAsynchronous() {
+  return false;
+}
+#endif  // !defined(OS_WIN)
+
 // static
 bool ShellIntegration::SetAsDefaultProtocolClientInteractive(
     const std::string& protocol) {
   return false;
 }
 
 // static
 bool ShellIntegration::IsElevationNeededForSettingDefaultProtocolClient() {
   return false;
 }
 
 #endif  // !defined(OS_WIN)
 
 bool ShellIntegration::DefaultWebClientObserver::IsOwnedByWorker() {
   return false;
 }
 
 bool ShellIntegration::DefaultWebClientObserver::
     IsInteractiveSetDefaultPermitted() {
   return false;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // ShellIntegration::DefaultWebClientWorker
 //
 
 ShellIntegration::DefaultWebClientWorker::DefaultWebClientWorker(
     DefaultWebClientObserver* observer)
-    : observer_(observer) {
-}
+    : observer_(observer) {}
 
 void ShellIntegration::DefaultWebClientWorker::StartCheckIsDefault() {
   if (observer_) {
     observer_->SetDefaultWebClientUIState(STATE_PROCESSING);
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
-        base::Bind(
-            &DefaultWebClientWorker::ExecuteCheckIsDefault, this));
+        base::Bind(&DefaultWebClientWorker::CheckIsDefault, this));
   }
 }
 
 void ShellIntegration::DefaultWebClientWorker::StartSetAsDefault() {
+  // Cancel the already running process if another start is requested.
+  if (set_as_default_in_progress_)

[grt (UTC plus 2), 2015/09/30 17:22:32]

in looking at how set_as_default_in_progress_ is used, it seems to be used for
two distinct things: 1) ignore successive calls to
OnSetAsDefaultAttemptComplete() after the initial one, and 2) ensure that
FinalizeSetAsDefault is called when there's an outstanding call to
InitializeSetAsDefault. for this latter case ("did something happen in
InitializeSetAsDefault that needs to be cleaned up"), i think a few small
changes would improve the interaction between DefaultWebClientWorker and
DefaultBrowserWorker since, for these cases, the followon work done by
OnSetAsDefaultAttemptComplete to check for defaultness isn't appropriate.

1) InitializeSetAsDefault() should return a bool indicating "i did work that
needs a subsequent call to FinalizeSetAsDefault()".
DefaultBrowserWorker::InitializeSetAsDefault would return false for all of its
early-exit codepaths.

2) a new bool set_as_default_initialized_ should be set on line 189 based on the
return value of InitializeSetAsDefault().

3) here, ObserverDestroyed(), and ~DefaultBrowserWorker() should be changed to
"if (set_as_default_initialized_) { set_as_default_initialized_ = false;
FinalizeSetAsDefault(false); }"

4) OnSetAsDefaultAttemptComplete should do similarly.

The idea here is to ensure that FinalizeSetAsDefault is called exactly once for
every successful call to InitializeSetAsDefault. Feel free to push back if I'm
missing something, or you think this is going in the wrong direction.

Is there a need to clear set_as_default_in_progress_ in ObserverDestroyed()?

[Patrick Monette, 2015/10/01 21:12:50]

Done.

+    OnSetAsDefaultAttemptComplete(false);
+
+  set_as_default_in_progress_ = true;
   bool interactive_permitted = false;
   if (observer_) {
     observer_->SetDefaultWebClientUIState(STATE_PROCESSING);
     interactive_permitted = observer_->IsInteractiveSetDefaultPermitted();
+
+    InitializeSetAsDefault();
   }
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&DefaultWebClientWorker::ExecuteSetAsDefault, this,
-                 interactive_permitted));
+
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                          base::Bind(&DefaultWebClientWorker::SetAsDefault,
+                                     this, interactive_permitted));
 }
 
 void ShellIntegration::DefaultWebClientWorker::ObserverDestroyed() {
   // Our associated view has gone away, so we shouldn't call back to it if
   // our worker thread returns after the view is dead.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   observer_ = NULL;
+  // If an attempt to set the default browser is already in progress, its result
+  // won't be posted to any observers. Manually invoke
+  // OnSetAsDefaultAttemptComplete() to ensure we free the default browser

[grt (UTC plus 2), 2015/09/30 17:22:32]

nit: "free" -> "clear"?

[Patrick Monette, 2015/10/01 21:12:51]

Done.

+  // callback and timer.
+  if (set_as_default_in_progress_)
+    OnSetAsDefaultAttemptComplete(false);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultWebClientWorker, private:
 
-void ShellIntegration::DefaultWebClientWorker::ExecuteCheckIsDefault() {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  DefaultWebClientState state = CheckIsDefault();
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &DefaultWebClientWorker::CompleteCheckIsDefault, this, state));
-}
+ShellIntegration::DefaultWebClientWorker::~DefaultWebClientWorker() {}
 
-void ShellIntegration::DefaultWebClientWorker::CompleteCheckIsDefault(
+void ShellIntegration::DefaultWebClientWorker::OnCheckIsDefaultComplete(
     DefaultWebClientState state) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   UpdateUI(state);
   // The worker has finished everything it needs to do, so free the observer
   // if we own it.
   if (observer_ && observer_->IsOwnedByWorker()) {
     delete observer_;
     observer_ = NULL;
   }
 }
 
-void ShellIntegration::DefaultWebClientWorker::ExecuteSetAsDefault(
-    bool interactive_permitted) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+void ShellIntegration::DefaultWebClientWorker::OnSetAsDefaultAttemptComplete(
+    bool succeeded) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // Hold on to a reference because if this was called via the default browser
+  // callback in StartupBrowserCreator, clearing the callback in
+  // FinalizeSetAsDefault would otherwise remove the last reference and delete
+  // us in the middle of this function.
+  scoped_refptr<DefaultWebClientWorker> scoped_ref(this);
 
-  bool result = SetAsDefault(interactive_permitted);
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&DefaultWebClientWorker::CompleteSetAsDefault, this, result));
+  if (set_as_default_in_progress_) {
+    set_as_default_in_progress_ = false;
+
+    FinalizeSetAsDefault(succeeded);
+    if (observer_)
+      observer_->OnSetAsDefaultConcluded(succeeded);
+
+    // Start the default browser check which will notify the observer as to
+    // whether Chrome is really the default browser. This is needed because
+    // detecting that the process was successful is not 100% sure.
+    // For example, on Windows 10+, the user might have unchecked the "Always
+    // use this app" checkbox which can't be detected.
+    StartCheckIsDefault();
+  }
 }
 
-void ShellIntegration::DefaultWebClientWorker::CompleteSetAsDefault(
-    bool succeeded) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  // First tell the observer what the SetAsDefault call has returned.
-  if (observer_)
-    observer_->OnSetAsDefaultConcluded(succeeded);
-  // Set as default completed, check again to make sure it stuck...
-  StartCheckIsDefault();
-}
+void ShellIntegration::DefaultWebClientWorker::InitializeSetAsDefault() {}
+
+void ShellIntegration::DefaultWebClientWorker::FinalizeSetAsDefault(
+    bool succeeded) {}
 
 void ShellIntegration::DefaultWebClientWorker::UpdateUI(
     DefaultWebClientState state) {
   if (observer_) {
     switch (state) {
       case NOT_DEFAULT:
         observer_->SetDefaultWebClientUIState(STATE_NOT_DEFAULT);
         break;
       case IS_DEFAULT:
         observer_->SetDefaultWebClientUIState(STATE_IS_DEFAULT);
         break;
       case UNKNOWN_DEFAULT:
         observer_->SetDefaultWebClientUIState(STATE_UNKNOWN);
         break;
       default:
         break;
     }
   }
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // ShellIntegration::DefaultBrowserWorker
 //
 
 ShellIntegration::DefaultBrowserWorker::DefaultBrowserWorker(
     DefaultWebClientObserver* observer)
     : DefaultWebClientWorker(observer) {
 }
 
+ShellIntegration::DefaultBrowserWorker::~DefaultBrowserWorker() {
+  if (set_as_default_in_progress_)
+    OnSetAsDefaultAttemptComplete(false);

[grt (UTC plus 2), 2015/09/30 17:22:32]

i don't think this could ever happen. the object can't be destroyed while either
async_timer_ is alive or the callback in SBC is set since each of those holds a
reference to the worker. i think this is reasonable:

  // The worker cannot be destroyed while an operation is in progress since all
  // tasks, timers, and callbacks hold a reference to it.
  DCHECK(!set_as_default_in_progress_);

wdyt? am i missing another possible destruction path?

[Patrick Monette, 2015/10/01 21:12:50]

SGTM. Done.

+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultBrowserWorker, private:
 
-ShellIntegration::DefaultWebClientState
-ShellIntegration::DefaultBrowserWorker::CheckIsDefault() {
-  return ShellIntegration::GetDefaultBrowser();
+void ShellIntegration::DefaultBrowserWorker::CheckIsDefault() {
+  DefaultWebClientState state = GetDefaultBrowser();
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultBrowserWorker::OnCheckIsDefaultComplete, this, state));
 }
 
-bool ShellIntegration::DefaultBrowserWorker::SetAsDefault(
+void ShellIntegration::DefaultBrowserWorker::SetAsDefault(
     bool interactive_permitted) {
   bool result = false;
-  switch (ShellIntegration::CanSetAsDefaultBrowser()) {
-    case ShellIntegration::SET_DEFAULT_UNATTENDED:
-      result = ShellIntegration::SetAsDefaultBrowser();
+  switch (CanSetAsDefaultBrowser()) {
+    case SET_DEFAULT_NOT_ALLOWED:
+      NOTREACHED();
       break;
-    case ShellIntegration::SET_DEFAULT_INTERACTIVE:
+    case SET_DEFAULT_UNATTENDED:
+      result = SetAsDefaultBrowser();
+      break;
+    case SET_DEFAULT_INTERACTIVE:
       if (interactive_permitted)
-        result = ShellIntegration::SetAsDefaultBrowserInteractive();
+        result = SetAsDefaultBrowserInteractive();
       break;
-    default:
+    case SET_DEFAULT_ASYNCHRONOUS:
+#if defined(OS_WIN)
+      if (!interactive_permitted)
+        break;
+      if (GetDefaultBrowser() == IS_DEFAULT) {
+        // Don't start the asynchronous operation since it could result in
+        // losing the default browser status.
+        result = true;
+        break;
+      }
+      // This function will cause OnSetAsDefaultAttemptComplete() to be called
+      // asynchronously via a filter established in InitializeSetAsDefault().
+      SetAsDefaultBrowserAsynchronous();
+      return;
+#else
       NOTREACHED();
+      break;
+#endif
   }
-
-  return result;
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultBrowserWorker::OnSetAsDefaultAttemptComplete, this,
+                 result));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // ShellIntegration::DefaultProtocolClientWorker
 //
 
 ShellIntegration::DefaultProtocolClientWorker::DefaultProtocolClientWorker(
     DefaultWebClientObserver* observer, const std::string& protocol)
     : DefaultWebClientWorker(observer),
       protocol_(protocol) {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultProtocolClientWorker, private:
 
-ShellIntegration::DefaultWebClientState
-ShellIntegration::DefaultProtocolClientWorker::CheckIsDefault() {
-  return ShellIntegration::IsDefaultProtocolClient(protocol_);
+ShellIntegration::DefaultProtocolClientWorker::~DefaultProtocolClientWorker() {}
+
+void ShellIntegration::DefaultProtocolClientWorker::CheckIsDefault() {
+  DefaultWebClientState state = IsDefaultProtocolClient(protocol_);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultProtocolClientWorker::OnCheckIsDefaultComplete, this,
+                 state));
 }
 
-bool ShellIntegration::DefaultProtocolClientWorker::SetAsDefault(
+void ShellIntegration::DefaultProtocolClientWorker::SetAsDefault(
     bool interactive_permitted) {
   bool result = false;
-  switch (ShellIntegration::CanSetAsDefaultProtocolClient()) {
-    case ShellIntegration::SET_DEFAULT_NOT_ALLOWED:
-      result = false;
+  switch (CanSetAsDefaultProtocolClient()) {
+    case SET_DEFAULT_NOT_ALLOWED:
+      // Not allowed, do nothing.
       break;
-    case ShellIntegration::SET_DEFAULT_UNATTENDED:
-      result = ShellIntegration::SetAsDefaultProtocolClient(protocol_);
+    case SET_DEFAULT_UNATTENDED:
+      result = SetAsDefaultProtocolClient(protocol_);
       break;
-    case ShellIntegration::SET_DEFAULT_INTERACTIVE:
+    case SET_DEFAULT_INTERACTIVE:
       if (interactive_permitted) {
-        result = ShellIntegration::SetAsDefaultProtocolClientInteractive(
-            protocol_);
+        result = SetAsDefaultProtocolClientInteractive(protocol_);
       }
       break;
+    case SET_DEFAULT_ASYNCHRONOUS:
+      NOTREACHED();
+      break;
   }
-
-  return result;
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultProtocolClientWorker::OnSetAsDefaultAttemptComplete,
+                 this, result));
 }