Prototype version of the first-run dialog for Windows 8

chrome/browser/shell_integration.cc

Index: chrome/browser/shell_integration.cc
diff --git a/chrome/browser/shell_integration.cc b/chrome/browser/shell_integration.cc
index 73754439a329da0826fd8123ea9c01e3d5dd5242..02bf6e6318b4f61db33b548462c33383712fef75 100644
--- a/chrome/browser/shell_integration.cc
+++ b/chrome/browser/shell_integration.cc
@@ -110,18 +110,25 @@ ShellIntegration::DefaultWebClientWorker::DefaultWebClientWorker(
 }
 
 void ShellIntegration::DefaultWebClientWorker::StartCheckIsDefault() {
+  StartCheckIsDefault(RESULT_SET_DEFAULT_UNDEFINED);
+}
+
+void ShellIntegration::DefaultWebClientWorker::StartCheckIsDefault(
+    SetDefaultWebClientResult call_result) {
   if (observer_) {
-    observer_->SetDefaultWebClientUIState(STATE_PROCESSING);
+    observer_->SetDefaultWebClientUIState(STATE_PROCESSING, call_result);
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
-        base::Bind(
-            &DefaultWebClientWorker::ExecuteCheckIsDefault, this));
+        base::Bind(&DefaultWebClientWorker::ExecuteCheckIsDefault,

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

chromium prefers to conserve vertical space in function calls.  please move
"this" up to this line.

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

Done.

+                   this,
+                   call_result));
   }
 }
 
 void ShellIntegration::DefaultWebClientWorker::StartSetAsDefault() {
   if (observer_) {
-    observer_->SetDefaultWebClientUIState(STATE_PROCESSING);
+    observer_->SetDefaultWebClientUIState(STATE_PROCESSING,
+                                          RESULT_SET_DEFAULT_UNDEFINED);
   }
   BrowserThread::PostTask(
       BrowserThread::FILE, FROM_HERE,
@@ -139,19 +146,22 @@ void ShellIntegration::DefaultWebClientWorker::ObserverDestroyed() {
 ///////////////////////////////////////////////////////////////////////////////
 // DefaultWebClientWorker, private:
 
-void ShellIntegration::DefaultWebClientWorker::ExecuteCheckIsDefault() {
+void ShellIntegration::DefaultWebClientWorker::ExecuteCheckIsDefault(
+    SetDefaultWebClientResult call_result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DefaultWebClientState state = CheckIsDefault();
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(
-          &DefaultWebClientWorker::CompleteCheckIsDefault, this, state));
+      base::Bind(&DefaultWebClientWorker::CompleteCheckIsDefault,

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

same note about condensing args

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

Done.

+                 this,
+                 state,
+                 call_result));
 }
 
 void ShellIntegration::DefaultWebClientWorker::CompleteCheckIsDefault(
-    DefaultWebClientState state) {
+    DefaultWebClientState state, SetDefaultWebClientResult call_result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  UpdateUI(state);
+  UpdateUI(state, call_result);
   // The worker has finished everything it needs to do, so free the observer
   // if we own it.
   if (observer_ && observer_->IsOwnedByWorker()) {
@@ -162,31 +172,34 @@ void ShellIntegration::DefaultWebClientWorker::CompleteCheckIsDefault(
 
 void ShellIntegration::DefaultWebClientWorker::ExecuteSetAsDefault() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  SetAsDefault(observer_ && observer_->IsInteractiveSetDefaultPermitted());
+  SetDefaultWebClientResult call_result = SetAsDefault(
+      observer_ && observer_->IsInteractiveSetDefaultPermitted());
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(
-          &DefaultWebClientWorker::CompleteSetAsDefault, this));
+          &DefaultWebClientWorker::CompleteSetAsDefault, this, call_result));
 }
 
-void ShellIntegration::DefaultWebClientWorker::CompleteSetAsDefault() {
+void ShellIntegration::DefaultWebClientWorker::CompleteSetAsDefault(
+    SetDefaultWebClientResult call_result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // Set as default completed, check again to make sure it stuck...
-  StartCheckIsDefault();
+  StartCheckIsDefault(call_result);

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

is there any reason to do this when call_result isn't RESULT_SET_DEFAULT_OK?

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

I think so. It may be a debug blindness, but I invoke it all the time while
*being* the default. 

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

do you mean that the browser choice dialog pops up when chrome is already the
default browser?  that seems bad.  we don't want to drive users to accidentally
clicking to make something other than chrome the default.

 }
 
 void ShellIntegration::DefaultWebClientWorker::UpdateUI(
-    DefaultWebClientState state) {
+    DefaultWebClientState state,
+    SetDefaultWebClientResult call_result) {
   if (observer_) {
     switch (state) {
       case NOT_DEFAULT_WEB_CLIENT:
-        observer_->SetDefaultWebClientUIState(STATE_NOT_DEFAULT);
+        observer_->SetDefaultWebClientUIState(STATE_NOT_DEFAULT, call_result);
         break;
       case IS_DEFAULT_WEB_CLIENT:
-        observer_->SetDefaultWebClientUIState(STATE_IS_DEFAULT);
+        observer_->SetDefaultWebClientUIState(STATE_IS_DEFAULT, call_result);
         break;
       case UNKNOWN_DEFAULT_WEB_CLIENT:
-        observer_->SetDefaultWebClientUIState(STATE_UNKNOWN);
+        observer_->SetDefaultWebClientUIState(STATE_UNKNOWN, call_result);
         break;
       default:
         break;
@@ -211,19 +224,25 @@ ShellIntegration::DefaultBrowserWorker::CheckIsDefault() {
   return ShellIntegration::IsDefaultBrowser();
 }
 
-void ShellIntegration::DefaultBrowserWorker::SetAsDefault(
-    bool interactive_permitted) {
+ShellIntegration::SetDefaultWebClientResult
+    ShellIntegration::DefaultBrowserWorker::SetAsDefault(
+        bool interactive_permitted) {
+  SetDefaultWebClientResult result = RESULT_SET_DEFAULT_UNDEFINED;
   switch (ShellIntegration::CanSetAsDefaultBrowser()) {
     case ShellIntegration::SET_DEFAULT_UNATTENDED:
-      ShellIntegration::SetAsDefaultBrowser();
+      result = ShellIntegration::SetAsDefaultBrowser() ?
+          RESULT_SET_DEFAULT_OK : RESULT_SET_DEFAULT_FAILED_OR_CANCELLED;
       break;
     case ShellIntegration::SET_DEFAULT_INTERACTIVE:
-      if (interactive_permitted)
-        ShellIntegration::SetAsDefaultBrowserInteractive();
+      if (interactive_permitted) {
+        result = ShellIntegration::SetAsDefaultBrowserInteractive() ?
+            RESULT_SET_DEFAULT_OK : RESULT_SET_DEFAULT_FAILED_OR_CANCELLED;
+      }
       break;
     default:
       NOTREACHED();
   }
+  return result;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -244,7 +263,10 @@ ShellIntegration::DefaultProtocolClientWorker::CheckIsDefault() {
   return ShellIntegration::IsDefaultProtocolClient(protocol_);
 }
 
-void ShellIntegration::DefaultProtocolClientWorker::SetAsDefault(
-    bool interactive_permitted) {
-  ShellIntegration::SetAsDefaultProtocolClient(protocol_);
+ShellIntegration::SetDefaultWebClientResult
+    ShellIntegration::DefaultProtocolClientWorker::SetAsDefault(
+        bool interactive_permitted) {
+  if (ShellIntegration::SetAsDefaultProtocolClient(protocol_))
+    return RESULT_SET_DEFAULT_OK;
+  return RESULT_SET_DEFAULT_FAILED_OR_CANCELLED;

[gab, 2012/06/16 19:32:51]

I prefer the way you have it in the function above this one, i.e.:

return ShellIntegration::SetAsDefaultProtocolClient(protocol_) ?
    RESULT_SET_DEFAULT_OK : RESULT_SET_DEFAULT_FAILED_OR_CANCELLED;
                            

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

Done.

 }