web intents. Fix callbacks to restore tab management on service exit.

chrome/browser/ui/intents/web_intent_picker_controller.cc

 // 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.
 
 #include "chrome/browser/ui/intents/web_intent_picker_controller.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 181 matching lines @@
       uma_reporter_(new UMAReporter()),
       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),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
-      ALLOW_THIS_IN_INITIALIZER_LIST(timer_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(timer_factory_(this)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(dispatcher_factory_(this)) {
   content::NavigationController* controller =
       &tab_contents->web_contents()->GetController();
   registrar_.Add(this, content::NOTIFICATION_LOAD_START,
                  content::Source<content::NavigationController>(controller));
   registrar_.Add(this, chrome::NOTIFICATION_TAB_CLOSING,
                  content::Source<content::NavigationController>(controller));
 #if defined(TOOLKIT_VIEWS)
   cancelled_ = true;
 #endif
 }
 
 WebIntentPickerController::~WebIntentPickerController() {
 }
 
 // TODO(gbillock): combine this with ShowDialog.
 void WebIntentPickerController::SetIntentsDispatcher(
     content::WebIntentsDispatcher* intents_dispatcher) {
   intents_dispatcher_ = intents_dispatcher;
+  // TODO(gbillock): This is to account for multiple dispatches in the same tab.
+  // That is currently not a well-handled case, and this is a band-aid.
+  dispatcher_factory_.InvalidateWeakPtrs();

[please use gerrit instead, 2012/09/20 16:38:30]

When you say "not a well-handled case", do you mean that it crashes, that it
works 50% of the time, or that it does not look right?

If this is a band-aid, then what is the right solution? Let's add that
information to the comment, so that we can take care of this issue quickly when
needed.

[Greg Billock, 2012/09/20 16:45:19]

The right solution is a "ChromeIntentsDispatcher" that has the same
query-lifetime semantics as the WebIntentsDispatcher, and we do this
RegisterReplyNotification there, not on the picker controller. We know we need
that, but we want to make changes right now that are patchable to M23, and a big
refactor isn't.


On 2012/09/20 16:38:30, Rouslan Solomakhin wrote:

   intents_dispatcher_->RegisterReplyNotification(
       base::Bind(&WebIntentPickerController::OnSendReturnMessage,
-                 weak_ptr_factory_.GetWeakPtr()));
+                 dispatcher_factory_.GetWeakPtr()));
 
   // Initialize the reporting bucket.
   const webkit_glue::WebIntentData& intent = intents_dispatcher_->GetIntent();
   uma_bucket_ = web_intents::ToUMABucket(intent.action, intent.type);
 }
 
 // TODO(smckay): rename this "StartActivity".
 void WebIntentPickerController::ShowDialog(const string16& action,
                                            const string16& type) {
   ShowDialog(false);
@@ skipping 814 matching lines @@
   web_intents::RecordPickerShow(
       uma_bucket_, picker_model_->GetInstalledServiceCount());
   picker_shown_ = true;
 }
 
 void WebIntentPickerController::ClosePicker() {
   SetDialogState(kPickerHidden);
   if (picker_)
     picker_->Close();
 }