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

chrome/browser/shell_integration.h

Index: chrome/browser/shell_integration.h
diff --git a/chrome/browser/shell_integration.h b/chrome/browser/shell_integration.h
index c0e4491af74708c2b046874540e6fdb4dd300842..ebb43e913f1ff32cf328d6b6f5ecd30a846df699 100644
--- a/chrome/browser/shell_integration.h
+++ b/chrome/browser/shell_integration.h
@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
+#include "base/timer/timer.h"
 #include "ui/gfx/image/image_family.h"
 #include "url/gurl.h"
 
@@ -32,6 +33,21 @@ class ShellIntegration {
   // shown and this method returns true.
   static bool SetAsDefaultBrowserInteractive();
 
+  // Returns true if setting the default browser is an asynchronous operation.
+  // In practice, this is only true on Windows 10+.
+  static bool IsSetAsDefaultAsynchronous();
+
+  // Prompt the user to select the default browser by trying to open
+  // https://support.google.com/chrome?p=default_browser which is the "How to
+  // set Chrome as your default browser" help page. Only call this if
+  // |IsSetAsDefaultAsynchronous| is true.
+  //
+  // The caller should call SetAsyncSetAsDefaultFilterCallback to intercept the

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

this comment isn't necessary. i think it's better to say that this call provides
no indication as to whether or not the operation succeeded, and that the reader
is more likely to want to use DefaultBrowserWorker, which will invoke its
observer as appropriate.

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

Done.

+  // link navigation (See SetAsyncSetAsDefaultFilterCallback in
+  // startup_browser_creator.h). The caller should also set up a timer to
+  // represent an unsuccessful attempt after a certain amount of time.
+  static void SetAsDefaultBrowserAsynchronous();
+
   // Sets Chrome as the default client application for the given protocol
   // (only for the current user). Returns false if this operation fails.
   static bool SetAsDefaultProtocolClient(const std::string& protocol);
@@ -45,13 +61,19 @@ class ShellIntegration {
   static bool SetAsDefaultProtocolClientInteractive(
       const std::string& protocol);
 
-  // In Windows 8 a browser can be made default-in-metro only in an interactive
-  // flow. We will distinguish between two types of permissions here to avoid
-  // forcing the user into UI interaction when this should not be done.
+  // Windows 8 and Windows 10 introduced differents way to set the default
+  // browsers.
   enum DefaultWebClientSetPermission {
+    // The browser control policy disallow setting the default browser.

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

even i didn't know what this meant. :-)  i think a better comment would be
    // The browser distribution is not permitted to be made default.

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

Done.

     SET_DEFAULT_NOT_ALLOWED,
+    // This is the most common case where no special permission or interaction
+    // is required to set the default browser.
     SET_DEFAULT_UNATTENDED,
+    // On Windows 8, a browser can be made fault only in an interactive flow.

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

fault -> default

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

Done.

     SET_DEFAULT_INTERACTIVE,
+    // On Windows 10+, the set as default browser flow is still interactive but
+    // is it also asynchronous.
+    SET_DEFAULT_ASYNCHRONOUS,
   };
 
   // Returns requirements for making the running browser the user's default.
@@ -226,51 +248,45 @@ class ShellIntegration {
 
     virtual ~DefaultWebClientWorker() {}
 
-   private:
-    // Function that performs the check.
-    virtual DefaultWebClientState CheckIsDefault() = 0;
-
-    // Function that sets Chrome as the default web client. Returns false if
-    // the operation fails or has been cancelled by the user.
-    virtual bool SetAsDefault(bool interactive_permitted) = 0;
-
-    // Function that handles performing the check on the file thread. This
-    // function is posted as a task onto the file thread, where it performs
-    // the check. When the check has finished the CompleteCheckIsDefault
-    // function is posted to the UI thread, where the result is sent back to
-    // the observer.
-    void ExecuteCheckIsDefault();
-
-    // Function that handles setting Chrome as the default web client on the
-    // file thread. This function is posted as a task onto the file thread.
-    // Once it is finished the CompleteSetAsDefault function is posted to the
-    // UI thread which will check the status of Chrome as the default, and
-    // send this to the observer.
-    // |interactive_permitted| indicates if the routine is allowed to carry on
-    // in context where user interaction is required (CanSetAsDefault*
-    // returns SET_DEFAULT_INTERACTIVE).
-    void ExecuteSetAsDefault(bool interactive_permitted);
-
-    // Communicate results to the observer. This function is posted as a task
-    // onto the UI thread by the ExecuteCheckIsDefault function running in the
-    // file thread.
+    // Communicate results to the observer. In contrast to CompleteSetAsDefault,
+    // this should not be called multiple times.
     void CompleteCheckIsDefault(DefaultWebClientState state);
 
-    // When the action to set Chrome as the default has completed this function
-    // is run. It is posted as a task back onto the UI thread by the
-    // ExecuteSetAsDefault function running in the file thread. This function
-    // will the start the check process, which, if an observer is present,
-    // reports to it the new status.
-    // |succeeded| is true if the actual call to a set-default function (from
-    // ExecuteSetAsDefault) was successful.
+    // Called when the set as default operation is finished. This then invokes
+    // FinalizeSetAsDefault and, if an observer is present, starts the check is
+    // default process. |succeeded| is true if the actual call to a set-default
+    // function was successful.
+    // It is safe to call this multiple times. Only the first call is processed
+    // after StartSetAsDefault is invoked.
     void CompleteSetAsDefault(bool succeeded);
 
+   private:
+    // Function that performs the check. Subclasses are responsible for calling

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

nit: mention that this is run on the FILE thread.

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

Done.

+    // CompleteCheckIsDefault on the UI thread.
+    virtual void CheckIsDefault() = 0;
+
+    // Sets Chrome as the default web client. Always called on the file thread.
+    // |interactive_permitted| will make SetAsDefault fails if it requires
+    // interaction with the user. Subclasses are responsible for calling
+    // CompleteSetAsDefault on the UI thread.
+    virtual void SetAsDefault(bool interactive_permitted) = 0;
+
+    // Invoked on the UI thread prior to starting a SetAsDefault operation.
+    virtual void InitializeSetAsDefault() {}
+
+    // Invoked on the UI thread following a SetAsDefault operation.
+    virtual void FinalizeSetAsDefault(bool succeeded) {}
+
     // Updates the UI in our associated view with the current default web
     // client state.
     void UpdateUI(DefaultWebClientState state);
 
     DefaultWebClientObserver* observer_;
 
+    // Flag that indicates if the SetAsDefault operation was completed to
+    // prevent multiple completed notifications to the observer.
+    bool set_as_default_completed_;

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

    bool set_as_default_completed_ = true;
and remove this member from the ctor's initializer list in the .cc file

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

Done.

+
     DISALLOW_COPY_AND_ASSIGN(DefaultWebClientWorker);
   };
 
@@ -283,10 +299,25 @@ class ShellIntegration {
     ~DefaultBrowserWorker() override {}
 
     // Check if Chrome is the default browser.
-    DefaultWebClientState CheckIsDefault() override;
+    void CheckIsDefault() override;
 
     // Set Chrome as the default browser.
-    bool SetAsDefault(bool interactive_permitted) override;
+    void SetAsDefault(bool interactive_permitted) override;
+
+    // On windows 10+, add the URL filter and starts the timer that determines
+    // if the operation was successful or not.
+    void InitializeSetAsDefault() override;
+
+    // On windows 10+, remove the URL filter and stops the timer.
+    void FinalizeSetAsDefault(bool succeeded) override;
+
+#if defined(OS_WIN)
+    // Used to determine if setting the default browser was unsuccesful.
+    scoped_ptr<base::OneShotTimer<DefaultBrowserWorker>> async_timer_;

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

the OneShotTimer tweak landed at r350496; please rebase. i think you can move
the #include of timer.h from here to the .cc file and forward declare
OneShotTimer now.

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

Done.

+
+    // Records the time it takes to set the default browser asynchronously.
+    base::TimeTicks start_time_;
+#endif  // !defined(OS_WIN)
 
     DISALLOW_COPY_AND_ASSIGN(DefaultBrowserWorker);
   };
@@ -307,10 +338,10 @@ class ShellIntegration {
 
    private:
     // Check is Chrome is the default handler for this protocol.
-    DefaultWebClientState CheckIsDefault() override;
+    void CheckIsDefault() override;
 
     // Set Chrome as the default handler for this protocol.
-    bool SetAsDefault(bool interactive_permitted) override;
+    void SetAsDefault(bool interactive_permitted) override;
 
     std::string protocol_;