Prototype version of the first-run dialog for Windows 8

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 eb7d716c6e81a1cfdf6b450fbc80e300f657fee4..b14b5694ccaa8211152cd8bc84dec24f8486a894 100644
--- a/chrome/browser/shell_integration.h
+++ b/chrome/browser/shell_integration.h
@@ -158,11 +158,19 @@ class ShellIntegration {
     STATE_UNKNOWN
   };
 
+  enum SetDefaultWebClientResult {

[grt (UTC plus 2), 2012/06/18 19:19:13]

Please document this.

[motek., 2012/06/19 18:06:44]

Done.

+    RESULT_SET_DEFAULT_OK,
+    RESULT_SET_DEFAULT_FAILED_OR_CANCELLED,
+    RESULT_SET_DEFAULT_UNDEFINED,
+  };
+
   class DefaultWebClientObserver {
    public:
     virtual ~DefaultWebClientObserver() {}
     // Updates the UI state to reflect the current default browser state.
-    virtual void SetDefaultWebClientUIState(DefaultWebClientUIState state) = 0;
+    virtual void SetDefaultWebClientUIState(

[grt (UTC plus 2), 2012/06/18 19:19:13]

i don't really like this as-is, since it confuses things for users of the
observe that only care about whether chrome is or isn't the default.

would the addition of STATE_REFUSED_DEFAULT to DefaultWebClientUIState be
sufficient?

[motek., 2012/06/19 18:06:44]

I did consider a similar idea. The trouble is that 'refused default' is not a
state, but a result. At the end of the sequence, refused or not, we can still be
default (if invoked when we are default already), not default or unknown. 
Moreover, this observer is used both in context of setting the default (which is
then followed by verification) and when we just want to check the state. I
thought it would be less surprising (albeit a bit more cumbersome indeed) to
pass both state and result as separate variables, with a specific result of
value to indicate no 'set' operation was undertaken.

I've also considered following options:
A. Turning DefaultWebClientUIState into a flag and composing state with result. 
B. Leaving this stuff as is and making a one-off for metroizer, where I would  
invoke both operations as wanted them. 
C. Adding a separate callback function to DefaultWebClientObserver to handle set
step (this would require a bit of synchronization work, though). 
D. Short-circuiting the whole thing on failure of the set operation. The trouble
I have with that is that this would effectively change the behaviour of this
operation in all other contexts (now after _PROCESSING you are bound to get
something else that makes sense, after the change - not so much; so, to display
the real state, say in options, I would need to ask again). 

If you really dislike this operation as is (personally, I think it makes sense),
I could do A or B (C is rather unappealing, because it wouldn't really buy as
much). What is your take?

[grt (UTC plus 2), 2012/06/19 20:43:09]

I was interpreting "state" in the sense of a FSM. "PROCESSING" is an
intermediate state, and "tried to set default but was rejected" is a terminal
state (this appears to be the what MetroizeHandler::SetDefaultWebClientUIState
cares about).  Does this seem any more palatable to you?
As I mentioned in another comment, I think we shouldn't bring up the choice
dialog if Chrome is already the default browser.  Are we?  Maybe I'm missing
something and it's not so bad after all, but it seems bad.
Would it?  The observer is notified on the UI thread, so you could add something
like this to CompleteSetAsDefault:

if (observer_)
  observer_->WhateverNewFunction(call_result);
C would avoid changing all other consumers of this API for the one that cares.

Would you mind tracking down the author of this WebClientWorker stuff and asking
them for their thoughts on the matter?

+        DefaultWebClientUIState state,
+        SetDefaultWebClientResult call_result) = 0;
     // Observer classes that return true to OwnedByWorker are automatically
     // freed by the worker when they are no longer needed.
     virtual bool IsOwnedByWorker() { return false; }
@@ -204,14 +212,19 @@ class ShellIntegration {
     virtual DefaultWebClientState CheckIsDefault() = 0;
 
     // Function that sets Chrome as the default web client.
-    virtual void SetAsDefault(bool interactive_permitted) = 0;
+    virtual SetDefaultWebClientResult SetAsDefault(
+        bool interactive_permitted) = 0;
+
+    // Private version takes a call result to be eventually forwarded to the
+    // observer.
+    void StartCheckIsDefault(SetDefaultWebClientResult call_result);
 
     // 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();
+    void ExecuteCheckIsDefault(SetDefaultWebClientResult call_result);
 
     // 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.
@@ -223,18 +236,20 @@ class ShellIntegration {
     // 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.
-    void CompleteCheckIsDefault(DefaultWebClientState state);
+    void CompleteCheckIsDefault(DefaultWebClientState state,
+                                SetDefaultWebClientResult call_result);
 
     // 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.
-    void CompleteSetAsDefault();
+    void CompleteSetAsDefault(SetDefaultWebClientResult call_result);
 
     // Updates the UI in our associated view with the current default web
     // client state.
-    void UpdateUI(DefaultWebClientState state);
+    void UpdateUI(DefaultWebClientState state,
+                  SetDefaultWebClientResult call_result);
 
     DefaultWebClientObserver* observer_;
 
@@ -253,7 +268,8 @@ class ShellIntegration {
     virtual DefaultWebClientState CheckIsDefault() OVERRIDE;
 
     // Set Chrome as the default browser.
-    virtual void SetAsDefault(bool interactive_permitted) OVERRIDE;
+    virtual SetDefaultWebClientResult SetAsDefault(
+        bool interactive_permitted) OVERRIDE;
 
     DISALLOW_COPY_AND_ASSIGN(DefaultBrowserWorker);
   };
@@ -277,7 +293,8 @@ class ShellIntegration {
     virtual DefaultWebClientState CheckIsDefault() OVERRIDE;
 
     // Set Chrome as the default handler for this protocol.
-    virtual void SetAsDefault(bool interactive_permitted) OVERRIDE;
+    virtual SetDefaultWebClientResult SetAsDefault(
+        bool interactive_permitted) OVERRIDE;
 
     std::string protocol_;