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..1e7507b849831ee58d2532ddc3bbd2a4aad31d62 100644
--- a/chrome/browser/shell_integration.cc
+++ b/chrome/browser/shell_integration.cc
@@ -110,18 +110,24 @@ 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, 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 +145,20 @@ 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, 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 +169,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());

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

apologies for not noticing this on the previous review, but this call to a
virtual function on the observer is taking place on the FILE thread.  all others
are on the UI thread.  would you move this call into StartSetAsDefault (which is
on the UI thread) and pass the bool into ExecuteSetAsDefault?

[motek., 2012/06/20 16:02:15]

Done.

   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));

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

how about making a member variable to hold call_result and setting it here (on
the UI thread) so it doesn't need to be passed to StartCheckIsDefault,
ExecuteCheckIsDefault, and CompleteCheckIsDefault?

[motek., 2012/06/20 16:02:15]

I went a different way after all (with a separate callback). This way client
worries about everything, seems simplest.

   // Set as default completed, check again to make sure it stuck...
-  StartCheckIsDefault();
+  StartCheckIsDefault(call_result);
 }
 
 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 +221,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 +260,9 @@ 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) {
+  return ShellIntegration::SetAsDefaultProtocolClient(protocol_) ?
+      RESULT_SET_DEFAULT_OK : RESULT_SET_DEFAULT_FAILED_OR_CANCELLED;
 }