Prototype version of the first-run dialog for Windows 8

chrome/browser/shell_integration.h

 // 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.
 
 #ifndef CHROME_BROWSER_SHELL_INTEGRATION_H_
 #define CHROME_BROWSER_SHELL_INTEGRATION_H_
 #pragma once
 
 #include <string>
 
@@ skipping 140 matching lines @@
   // querying if Chrome is the default browser or the default handler
   // application for a url protocol, and communicates the state and result of
   // a request.
   enum DefaultWebClientUIState {
     STATE_PROCESSING,
     STATE_NOT_DEFAULT,
     STATE_IS_DEFAULT,
     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; }
     // An observer can permit or decline set-as-default operation if it
     // requires triggering user interaction.
     virtual bool IsInteractiveSetDefaultPermitted() { return false; }
   };
 
   //  Helper objects that handle checking if Chrome is the default browser
   //  or application for a url protocol on Windows and Linux, and also setting
@@ skipping 21 matching lines @@
    protected:
     friend class base::RefCountedThreadSafe<DefaultWebClientWorker>;
 
     virtual ~DefaultWebClientWorker() {}
 
    private:
     // Function that performs the check.
     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.
     // 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.
     void ExecuteSetAsDefault();
 
     // 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_;
 
     DISALLOW_COPY_AND_ASSIGN(DefaultWebClientWorker);
   };
 
   // Worker for checking and setting the default browser.
   class DefaultBrowserWorker : public DefaultWebClientWorker {
    public:
     explicit DefaultBrowserWorker(DefaultWebClientObserver* observer);
 
    private:
     virtual ~DefaultBrowserWorker() {}
 
     // Check if Chrome is the default browser.
     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);
   };
 
   // Worker for checking and setting the default client application
   // for a given protocol. A different worker instance is needed for each
   // protocol you are interested in, so to check or set the default for
   // multiple protocols you should use multiple worker objects.
   class DefaultProtocolClientWorker : public DefaultWebClientWorker {
    public:
     DefaultProtocolClientWorker(DefaultWebClientObserver* observer,
                                 const std::string& protocol);
 
     const std::string& protocol() const { return protocol_; }
 
    protected:
     virtual ~DefaultProtocolClientWorker() {}
 
    private:
     // Check is Chrome is the default handler for this protocol.
     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_;
 
     DISALLOW_COPY_AND_ASSIGN(DefaultProtocolClientWorker);
   };
 };
 
 #endif  // CHROME_BROWSER_SHELL_INTEGRATION_H_