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"
@@ skipping 11 matching lines @@
 #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;
 
+#if !defined(OS_WIN)
+// static
+bool ShellIntegration::IsSetAsDefaultAsynchronous() {
+  return false;
+}
+
+// static
+void ShellIntegration::SetAsDefaultBrowserAsynchronous() {
+  NOTREACHED();
+}
+#endif  // !defined(OS_WIN)
+
 // static
 ShellIntegration::DefaultWebClientSetPermission
     ShellIntegration::CanSetAsDefaultProtocolClient() {
   // Allowed as long as the browser can become the operating system default
   // browser.
-  return CanSetAsDefaultBrowser();
+  auto permission = CanSetAsDefaultBrowser();
+
+  // Set as default asynchronous is not supported for all protocols.

[grt (UTC plus 2), 2015/09/24 18:24:11]

suggestion:
"not supported for all protocols" -> "only supported for default web browser"

[Patrick Monette, 2015/09/25 20:06:24]

Done.

+  if (permission == SET_DEFAULT_ASYNCHRONOUS)
+    permission = SET_DEFAULT_INTERACTIVE;

[grt (UTC plus 2), 2015/09/24 18:24:11]

nit:
  if (blah)
    return SET_DEFAULT_INTERACTIVE;
  return permission;
or rename "permission" to "perm" and:
  return (perm == SET_DEFAULT_ASYNCHRONOUS) ? SET_DEFAULT_INTERACTIVE : perm;

[Patrick Monette, 2015/09/25 20:06:24]

Done.

+
+  return permission;
 }
 
 static const struct ShellIntegration::AppModeInfo* gAppModeInfo = NULL;
 
 // static
 void ShellIntegration::SetAppModeInfo(const struct AppModeInfo* info) {
   gAppModeInfo = info;
 }
 
 // static
@@ skipping 97 matching lines @@
     IsInteractiveSetDefaultPermitted() {
   return false;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // ShellIntegration::DefaultWebClientWorker
 //
 
 ShellIntegration::DefaultWebClientWorker::DefaultWebClientWorker(
     DefaultWebClientObserver* observer)
-    : observer_(observer) {
-}
+    : observer_(observer), set_as_default_completed_(true) {}
 
 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() {
+  set_as_default_completed_ = false;

[grt (UTC plus 2), 2015/09/24 18:24:11]

should this handle the case where a previous StartSetAsDefault hasn't yet
completed? maybe FinalizeSetAsDefault(false) so that the old filter callback is
removed? or have InitializeSetAsDefault() be a noop if the timer already exists?

[Patrick Monette, 2015/09/25 20:06:24]

Calling FinalizeSetAsDefault is better imo because we want to try again if the
user repeatedly click Set default browser. The intent picker might have
disappeared and we are waiting on the timer to finish.

   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 set as default is currently running, CompleteSetAsDefault is invoked as
+  // the result will no longer be posted to any observers and we want to clear
+  // up ressources.

[grt (UTC plus 2), 2015/09/24 18:24:11]

nit: resources

[Patrick Monette, 2015/09/25 20:06:24]

Done.

+  if (!set_as_default_completed_)
+    CompleteSetAsDefault(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));
-}
-
 void ShellIntegration::DefaultWebClientWorker::CompleteCheckIsDefault(
     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);
-
-  bool result = SetAsDefault(interactive_permitted);
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&DefaultWebClientWorker::CompleteSetAsDefault, this, result));
-}
-
 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();
+  if (!set_as_default_completed_) {

[grt (UTC plus 2), 2015/09/24 18:24:11]

wdyt of set_as_default_in_progress_ instead of set_as_default_completed_ so that
it's false to begin with and true when something is happening? it makes this
conditional more clear, to boot.

[Patrick Monette, 2015/09/25 20:06:24]

Done.

+    FinalizeSetAsDefault(succeeded);
+    if (observer_)
+      observer_->OnSetAsDefaultConcluded(succeeded);
+    // Set as default completed, check again to make sure it stuck...
+    StartCheckIsDefault();
+  }
+  set_as_default_completed_ = true;

[grt (UTC plus 2), 2015/09/24 18:24:11]

nit: move this up into the body of the if()

[Patrick Monette, 2015/09/25 20:06:24]

Done.

 }
 
 void ShellIntegration::DefaultWebClientWorker::UpdateUI(
     DefaultWebClientState state) {
   if (observer_) {
     switch (state) {
       case NOT_DEFAULT:
         observer_->SetDefaultWebClientUIState(STATE_NOT_DEFAULT);
         break;
       case IS_DEFAULT:
@@ skipping 13 matching lines @@
 //
 
 ShellIntegration::DefaultBrowserWorker::DefaultBrowserWorker(
     DefaultWebClientObserver* observer)
     : DefaultWebClientWorker(observer) {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultBrowserWorker, private:
 
-ShellIntegration::DefaultWebClientState
-ShellIntegration::DefaultBrowserWorker::CheckIsDefault() {
-  return ShellIntegration::GetDefaultBrowser();
+void ShellIntegration::DefaultBrowserWorker::CheckIsDefault() {
+  DefaultWebClientState state = ShellIntegration::GetDefaultBrowser();
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultBrowserWorker::CompleteCheckIsDefault, this, state));
 }
 
-bool ShellIntegration::DefaultBrowserWorker::SetAsDefault(
+void ShellIntegration::DefaultBrowserWorker::SetAsDefault(
     bool interactive_permitted) {
   bool result = false;
   switch (ShellIntegration::CanSetAsDefaultBrowser()) {
+    case ShellIntegration::SET_DEFAULT_NOT_ALLOWED:
+      NOTREACHED();
+      break;
     case ShellIntegration::SET_DEFAULT_UNATTENDED:
       result = ShellIntegration::SetAsDefaultBrowser();
       break;
     case ShellIntegration::SET_DEFAULT_INTERACTIVE:
-      if (interactive_permitted)
+      if (interactive_permitted) {

[grt (UTC plus 2), 2015/09/24 18:24:11]

nit: omit braces

[Patrick Monette, 2015/09/25 20:06:24]

Done.

         result = ShellIntegration::SetAsDefaultBrowserInteractive();
+      }
       break;
-    default:
-      NOTREACHED();
+    case ShellIntegration::SET_DEFAULT_ASYNCHRONOUS:
+      if (interactive_permitted) {
+        // This function will cause CompleteSetAsDefault to be called
+        // asynchronously through a filter in startup_browser_creator.h.

[grt (UTC plus 2), 2015/09/24 18:24:11]

suggestion:
        // asynchronously via a filter established in InitializeSetAsDefault().

[Patrick Monette, 2015/09/25 20:06:24]

Done.

+        ShellIntegration::SetAsDefaultBrowserAsynchronous();
+        return;
+      }
+      break;
   }
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultBrowserWorker::CompleteSetAsDefault, this, result));
+}
 
-  return result;
-}
+#if !defined(OS_WIN)
+void ShellIntegration::DefaultBrowserWorker::InitializeSetAsDefault() {}
+
+void ShellIntegration::DefaultBrowserWorker::FinalizeSetAsDefault(
+    bool succeeded) {}
+#endif  // !defined(OS_WIN)
 
 ///////////////////////////////////////////////////////////////////////////////
 // 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_);
+void ShellIntegration::DefaultProtocolClientWorker::CheckIsDefault() {
+  DefaultWebClientState state =
+      ShellIntegration::IsDefaultProtocolClient(protocol_);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&DefaultProtocolClientWorker::CompleteCheckIsDefault, 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;
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(&DefaultProtocolClientWorker::CompleteSetAsDefault, this,
+                     false));
       break;
     case ShellIntegration::SET_DEFAULT_UNATTENDED:
       result = ShellIntegration::SetAsDefaultProtocolClient(protocol_);
+      BrowserThread::PostTask(
+          BrowserThread::UI, FROM_HERE,
+          base::Bind(&DefaultProtocolClientWorker::CompleteSetAsDefault, this,
+                     result));
       break;
     case ShellIntegration::SET_DEFAULT_INTERACTIVE:
       if (interactive_permitted) {
-        result = ShellIntegration::SetAsDefaultProtocolClientInteractive(
-            protocol_);
+        result =
+            ShellIntegration::SetAsDefaultProtocolClientInteractive(protocol_);
+        BrowserThread::PostTask(
+            BrowserThread::UI, FROM_HERE,
+            base::Bind(&DefaultProtocolClientWorker::CompleteSetAsDefault, this,
+                       result));
       }
       break;
+    case ShellIntegration::SET_DEFAULT_ASYNCHRONOUS:
+      NOTREACHED();
+      break;
   }
-
-  return result;
 }