[WebIntents, Gtk] "Waiting for Suggestion" dialog

chrome/browser/ui/intents/web_intent_picker_controller.cc

Index: chrome/browser/ui/intents/web_intent_picker_controller.cc
diff --git a/chrome/browser/ui/intents/web_intent_picker_controller.cc b/chrome/browser/ui/intents/web_intent_picker_controller.cc
index 38eb5f2e0019f76a62e0aa1adc8e8e561609b313..e08388abbf68dcec1b8c6dfa2d897a326756e24f 100644
--- a/chrome/browser/ui/intents/web_intent_picker_controller.cc
+++ b/chrome/browser/ui/intents/web_intent_picker_controller.cc
@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/browser/ui/constrained_window_tab_helper.h"
 #include "chrome/browser/ui/intents/web_intent_picker.h"
 #include "chrome/browser/ui/intents/web_intent_picker_model.h"
 #include "chrome/browser/ui/tab_contents/tab_contents.h"
@@ -60,6 +61,13 @@ const char kPickActionURL[] = "http://webintents.org/pick";
 const char kSubscribeActionURL[] = "http://webintents.org/subscribe";
 const char kSaveActionURL[] = "http://webintents.org/save";
 
+// Maximum amount of time to delay displaying dialog while waiting for data.
+const int kMaxHiddenSetupTimeMs = 200;
+
+// Minimum amount of time to show waiting dialog, if it is shown.
+const int kMinThrobberDisplayTimeMs = 2000;
+
+
 // Gets the favicon service for the profile in |tab_contents|.
 FaviconService* GetFaviconService(TabContents* tab_contents) {
   return tab_contents->profile()->GetFaviconService(Profile::EXPLICIT_ACCESS);
@@ -161,17 +169,20 @@ class SourceWindowObserver : content::WebContentsObserver {
 
 WebIntentPickerController::WebIntentPickerController(
     TabContents* tab_contents)
-    : tab_contents_(tab_contents),
+    : dialog_state_(kPickerHidden),
+      tab_contents_(tab_contents),
       picker_(NULL),
       picker_model_(new WebIntentPickerModel()),
       pending_async_count_(0),
       pending_registry_calls_count_(0),
+      pending_cws_request_(false),
       picker_shown_(false),
       window_disposition_source_(NULL),
       source_intents_dispatcher_(NULL),
       intents_dispatcher_(NULL),
       service_tab_(NULL),
-      weak_ptr_factory_(this) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(timer_factory_(this)) {
   content::NavigationController* controller =
       &tab_contents->web_contents()->GetController();
   registrar_.Add(this, content::NOTIFICATION_LOAD_START,
@@ -191,9 +202,13 @@ void WebIntentPickerController::SetIntentsDispatcher(
       base::Bind(&WebIntentPickerController::OnSendReturnMessage,
                  weak_ptr_factory_.GetWeakPtr()));
 }
-
 void WebIntentPickerController::ShowDialog(const string16& action,
                                            const string16& type) {
+
+  // As soon as the dialog is requested, block all input events
+  // on the original tab.
+  tab_contents_->constrained_window_tab_helper()->BlockTabContent(true);
+
   // Only show a picker once.
   // TODO(gbillock): There's a hole potentially admitting multiple
   // in-flight dispatches since we don't create the picker
@@ -247,9 +262,13 @@ void WebIntentPickerController::ShowDialog(const string16& action,
     }
   }
 
+  SetDialogState(kPickerSetup);
+
   pending_async_count_ += 2;
   pending_registry_calls_count_ += 1;

[Greg Billock, 2012/08/08 23:47:11]

There are two of these -- services and defaults. Better to track them separately
with their own flags/events. Then we can figure out what's going on more easily.

[groby-ooo-7-16, 2012/08/09 21:59:56]

See discussion below - tempted to move to bitfield.

I might reserve that for a second refactoring, though. I really want to land
this functionality :)
On 2012/08/08 23:47:11, Greg Billock wrote:

 
+  pending_cws_request_ = true;
+
   GetWebIntentsRegistry(tab_contents_)->GetIntentServices(
       action, type,
           base::Bind(&WebIntentPickerController::OnWebIntentServicesAvailable,
@@ -265,10 +284,10 @@ void WebIntentPickerController::ShowDialog(const string16& action,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-    GetCWSIntentsRegistry(tab_contents_)->GetIntentServices(
-        action, type,
-        base::Bind(&WebIntentPickerController::OnCWSIntentServicesAvailable,
-                   weak_ptr_factory_.GetWeakPtr()));
+  GetCWSIntentsRegistry(tab_contents_)->GetIntentServices(
+      picker_model_->action(), picker_model_->mimetype(),
+      base::Bind(&WebIntentPickerController::OnCWSIntentServicesAvailable,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void WebIntentPickerController::Observe(
@@ -515,11 +534,7 @@ void WebIntentPickerController::WebIntentServicesForExplicitIntent(
         favicon_service, handle,
         picker_model_->GetInstalledServiceCount() - 1);
 
-    if (services[i].disposition ==
-        webkit_glue::WebIntentServiceData::DISPOSITION_INLINE)
-      CreatePicker();
-    OnServiceChosen(services[i].service_url,
-                    ConvertDisposition(services[i].disposition));
+    InvokeService(picker_model_->GetInstalledServiceAt(i));
     AsyncOperationFinished();
     return;
   }
@@ -555,18 +570,13 @@ void WebIntentPickerController::RegistryCallsCompleted() {
             GURL(picker_model_->default_service_url()));
 
     if (default_service != NULL) {
-      if (default_service->disposition ==
-          WebIntentPickerModel::DISPOSITION_INLINE)
-        CreatePicker();
-
-      OnServiceChosen(default_service->url, default_service->disposition);
+      InvokeService(*default_service);
       return;
     }
   }
 
-  CreatePicker();
-  picker_->SetActionString(GetIntentActionString(
-      UTF16ToUTF8(picker_model_->action())));
+  OnPickerEvent(kPickerEventRegistryData);
+  OnIntentDataArrived();
 }
 
 void WebIntentPickerController::OnFaviconDataAvailable(
@@ -621,6 +631,8 @@ void WebIntentPickerController::OnCWSIntentServicesAvailable(
   }
 
   AsyncOperationFinished();

[Greg Billock, 2012/08/08 23:47:11]

It'd be nice to use the state machine to kill this off -- just notify the state
machine on events, and let it figure out what state to go into.

[groby-ooo-7-16, 2012/08/09 21:59:56]

I'm trying to have the states correspond to actual action taken - this would
introduce a dummy state. 

And it'd be a _real_ pain once I moved the pending_registry_calls_count_ over -
we have three pieces of data coming in, so a total of 8 states. Only 3 of which
we're interested in.


On 2012/08/08 23:47:11, Greg Billock wrote:

+  pending_cws_request_ = false;
+  OnIntentDataArrived();
 }
 
 void WebIntentPickerController::OnExtensionIconURLFetchComplete(
@@ -668,6 +680,14 @@ void WebIntentPickerController::OnExtensionIconURLFetchComplete(
                  unavailable_callback));
 }
 
+void WebIntentPickerController::OnIntentDataArrived() {
+  DCHECK(picker_model_.get());
+
+  if (!pending_cws_request_ &&
+      pending_registry_calls_count_ == 0)

[Greg Billock, 2012/08/08 23:47:11]

Merge this into the state machine logic.

[groby-ooo-7-16, 2012/08/09 21:59:56]

See above. I'm treating this as a mini state machine. :)

Would using a bit field instead of a count make it more obvious?

On 2012/08/08 23:47:11, Greg Billock wrote:

+    OnPickerEvent(kPickerEventDataComplete);
+}
+
 // static
 void WebIntentPickerController::DecodeExtensionIconAndResize(
     scoped_ptr<std::string> icon_response,
@@ -756,6 +776,48 @@ void WebIntentPickerController::OnExtensionInstallServiceAvailable(
   AsyncOperationFinished();
 }
 
+void WebIntentPickerController::OnPickerEvent(WebIntentPickerEvent event) {
+  switch (event) {
+    case kPickerEventHiddenSetupTimeout:
+      DCHECK(dialog_state_ == kPickerSetup);
+      SetDialogState(kPickerWaiting);
+      break;
+
+    case kPickerEventMaxWaitTimeExceeded:
+      DCHECK(dialog_state_ == kPickerWaiting);
+
+      // If registry data is complete, go to main dialog. Otherwise, wait.
+      if (pending_registry_calls_count_ == 0)
+        SetDialogState(kPickerMain);
+      else
+        SetDialogState(kPickerWaitLong);
+      break;
+
+    case kPickerEventRegistryData:
+      DCHECK(dialog_state_ == kPickerSetup ||
+             dialog_state_ == kPickerWaiting ||
+             dialog_state_ == kPickerWaitLong);
+
+      // If minimum wait dialog time is exceeded, display main dialog.
+      // Either way, we don't do a thing.
+      break;
+
+    case kPickerEventDataComplete:
+      DCHECK(dialog_state_ == kPickerSetup ||
+              dialog_state_ == kPickerWaiting);
+
+      // In setup state, transition to main dialog. In waiting state, let
+      // timer expire.
+      if (dialog_state_ == kPickerSetup)
+        SetDialogState(kPickerMain);
+      break;
+
+    default:
+      NOTREACHED();
+      break;
+  }
+}
+
 void WebIntentPickerController::AsyncOperationFinished() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   if (--pending_async_count_ == 0) {
@@ -764,14 +826,82 @@ void WebIntentPickerController::AsyncOperationFinished() {
   }
 }
 
+void WebIntentPickerController::InvokeService(
+    const WebIntentPickerModel::InstalledService& service) {
+  if (service.disposition == WebIntentPickerModel::DISPOSITION_INLINE) {
+    SetDialogState(kPickerMain);
+  }
+  OnServiceChosen(service.url, service.disposition);
+}
+
+void WebIntentPickerController::SetDialogState(WebIntentPickerState state) {
+  // Ignore events that don't change state.
+  if (state == dialog_state_)
+    return;
+
+  // Any pending timers are abandoned on state changes.
+  timer_factory_.InvalidateWeakPtrs();
+
+  switch (state) {
+    case kPickerSetup:
+      DCHECK(dialog_state_ == kPickerHidden);
+
+      // Post timer CWS pending
+      MessageLoop::current()->PostDelayedTask(FROM_HERE,
+          base::Bind(&WebIntentPickerController::OnPickerEvent,
+                     timer_factory_.GetWeakPtr(),
+                     kPickerEventHiddenSetupTimeout),
+          base::TimeDelta::FromMilliseconds(kMaxHiddenSetupTimeMs));
+      break;
+
+    case kPickerWaiting:
+      DCHECK(dialog_state_ == kPickerSetup);
+      // Waiting dialog can be dismissed after minimum wait time.
+      MessageLoop::current()->PostDelayedTask(FROM_HERE,
+          base::Bind(&WebIntentPickerController::OnPickerEvent,
+                     timer_factory_.GetWeakPtr(),
+                     kPickerEventMaxWaitTimeExceeded),
+          base::TimeDelta::FromMilliseconds(kMinThrobberDisplayTimeMs));
+      break;
+
+    case kPickerWaitLong:
+      DCHECK(dialog_state_ == kPickerWaiting);
+      break;
+
+    case kPickerMain:
+      // No DCHECK - main state can be reached from any state.
+      // Ready to display data.
+      picker_model_->SetWaitingForSuggestions(false);
+      break;
+
+    case kPickerHidden:
+      break;
+
+    default:
+      NOTREACHED();
+      break;
+
+  }
+
+  dialog_state_ = state;
+
+  // Create picker dialog when changing away from hidden state.
+  if (dialog_state_ != kPickerHidden && dialog_state_ != kPickerSetup)
+    CreatePicker();
+}
+
+
 void WebIntentPickerController::CreatePicker() {
   // If picker is non-NULL, it was set by a test.
   if (picker_ == NULL)
     picker_ = WebIntentPicker::Create(tab_contents_, this, picker_model_.get());
+  picker_->SetActionString(GetIntentActionString(
+      UTF16ToUTF8(picker_model_->action())));
   picker_shown_ = true;
 }
 
 void WebIntentPickerController::ClosePicker() {
+  SetDialogState(kPickerHidden);
   if (picker_)
     picker_->Close();
 }