diff options
author | satish@chromium.org <satish@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-07 12:14:44 +0000 |
---|---|---|
committer | satish@chromium.org <satish@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-07 12:14:44 +0000 |
commit | 061c740bf01098700818a116eda2abac66ba2ee1 (patch) | |
tree | d9bdc45fe1cf06c45778bbbe0e1a4b2d73907bef /chrome | |
parent | f2593f76a25a0d327f7695e7eaf115e11a7da128 (diff) | |
download | chromium_src-061c740bf01098700818a116eda2abac66ba2ee1.zip chromium_src-061c740bf01098700818a116eda2abac66ba2ee1.tar.gz chromium_src-061c740bf01098700818a116eda2abac66ba2ee1.tar.bz2 |
Listen for tab close notifications and cancel active speech sessions.
BUG=none
TEST=Start speech input and close tab with keyboard before the UI comes up, check for crashes.
Review URL: http://codereview.chromium.org/6115001
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@70733 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
9 files changed, 132 insertions, 59 deletions
diff --git a/chrome/browser/speech/speech_input_bubble.cc b/chrome/browser/speech/speech_input_bubble.cc index 655415f..77619e9 100644 --- a/chrome/browser/speech/speech_input_bubble.cc +++ b/chrome/browser/speech/speech_input_bubble.cc @@ -32,9 +32,10 @@ SpeechInputBubble* SpeechInputBubble::Create(TabContents* tab_contents, return CreateNativeBubble(tab_contents, delegate, element_rect); } -SpeechInputBubbleBase::SpeechInputBubbleBase() +SpeechInputBubbleBase::SpeechInputBubbleBase(TabContents* tab_contents) : ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)), - display_mode_(DISPLAY_MODE_RECORDING) { + display_mode_(DISPLAY_MODE_RECORDING), + tab_contents_(tab_contents) { if (!mic_empty_) { // Static variables. mic_empty_ = ResourceBundle::GetSharedInstance().GetBitmapNamed( IDR_SPEECH_INPUT_MIC_EMPTY); diff --git a/chrome/browser/speech/speech_input_bubble.h b/chrome/browser/speech/speech_input_bubble.h index 3faabe0..436e6bb 100644 --- a/chrome/browser/speech/speech_input_bubble.h +++ b/chrome/browser/speech/speech_input_bubble.h @@ -99,6 +99,9 @@ class SpeechInputBubble { // Updates the current captured audio volume displayed on screen. virtual void SetInputVolume(float volume) = 0; + // Returns the TabContents for which this bubble gets displayed. + virtual TabContents* tab_contents() = 0; + // The horizontal distance between the start of the html widget and the speech // bubble's arrow. static const int kBubbleTargetOffsetX; @@ -119,7 +122,7 @@ class SpeechInputBubbleBase : public SpeechInputBubble { DISPLAY_MODE_MESSAGE }; - SpeechInputBubbleBase(); + explicit SpeechInputBubbleBase(TabContents* tab_contents); virtual ~SpeechInputBubbleBase(); // SpeechInputBubble methods @@ -127,6 +130,7 @@ class SpeechInputBubbleBase : public SpeechInputBubble { virtual void SetRecognizingMode(); virtual void SetMessage(const string16& text); virtual void SetInputVolume(float volume); + virtual TabContents* tab_contents() { return tab_contents_; } protected: // Updates the platform specific UI layout for the current display mode. @@ -159,6 +163,8 @@ class SpeechInputBubbleBase : public SpeechInputBubble { scoped_ptr<SkBitmap> mic_image_; // A temporary buffer image used in creating the above mic image. scoped_ptr<SkBitmap> buffer_image_; + // TabContents in which this this bubble gets displayed. + TabContents* tab_contents_; static SkBitmap* mic_full_; // Mic image with full volume. static SkBitmap* mic_empty_; // Mic image with zero volume. diff --git a/chrome/browser/speech/speech_input_bubble_controller.cc b/chrome/browser/speech/speech_input_bubble_controller.cc index 682d028..b048589 100644 --- a/chrome/browser/speech/speech_input_bubble_controller.cc +++ b/chrome/browser/speech/speech_input_bubble_controller.cc @@ -7,13 +7,17 @@ #include "chrome/browser/browser_thread.h" #include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/browser/tab_contents/tab_util.h" +#include "chrome/common/notification_registrar.h" +#include "chrome/common/notification_source.h" +#include "chrome/common/notification_type.h" #include "gfx/rect.h" namespace speech_input { SpeechInputBubbleController::SpeechInputBubbleController(Delegate* delegate) : delegate_(delegate), - current_bubble_caller_id_(0) { + current_bubble_caller_id_(0), + registrar_(new NotificationRegistrar) { } SpeechInputBubbleController::~SpeechInputBubbleController() { @@ -43,6 +47,8 @@ void SpeechInputBubbleController::CreateBubble(int caller_id, return; bubbles_[caller_id] = bubble; + + UpdateTabContentsSubscription(caller_id, BUBBLE_ADDED); } void SpeechInputBubbleController::CloseBubble(int caller_id) { @@ -70,6 +76,60 @@ void SpeechInputBubbleController::SetBubbleMessage(int caller_id, ProcessRequestInUiThread(caller_id, REQUEST_SET_MESSAGE, text, 0); } +void SpeechInputBubbleController::UpdateTabContentsSubscription( + int caller_id, ManageSubscriptionAction action) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + // If there are any other bubbles existing for the same TabContents, we would + // have subscribed to tab close notifications on their behalf and we need to + // stay registered. So we don't change the subscription in such cases. + TabContents* tab_contents = bubbles_[caller_id]->tab_contents(); + for (BubbleCallerIdMap::iterator iter = bubbles_.begin(); + iter != bubbles_.end(); ++iter) { + if (iter->second->tab_contents() == tab_contents && + iter->first != caller_id) { + // At least one other bubble exists for the same TabContents. So don't + // make any change to the subscription. + return; + } + } + + if (action == BUBBLE_ADDED) { + registrar_->Add(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source<TabContents>(tab_contents)); + } else { + registrar_->Remove(this, NotificationType::TAB_CONTENTS_DESTROYED, + Source<TabContents>(tab_contents)); + } +} + +void SpeechInputBubbleController::Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details) { + if (type == NotificationType::TAB_CONTENTS_DESTROYED) { + // Cancel all bubbles and active recognition sessions for this tab. + TabContents* tab_contents = Source<TabContents>(source).ptr(); + BubbleCallerIdMap::iterator iter = bubbles_.begin(); + while (iter != bubbles_.end()) { + if (iter->second->tab_contents() == tab_contents) { + BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, + NewRunnableMethod( + this, + &SpeechInputBubbleController::InvokeDelegateButtonClicked, + iter->first, SpeechInputBubble::BUTTON_CANCEL)); + CloseBubble(iter->first); + // We expect to have a very small number of items in this map so + // redo-ing from start is ok. + iter = bubbles_.begin(); + } else { + ++iter; + } + } + } else { + NOTREACHED() << "Unknown notification"; + } +} + void SpeechInputBubbleController::ProcessRequestInUiThread( int caller_id, RequestType type, const string16& text, float volume) { if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) { @@ -109,6 +169,7 @@ void SpeechInputBubbleController::ProcessRequestInUiThread( case REQUEST_CLOSE: if (current_bubble_caller_id_ == caller_id) current_bubble_caller_id_ = 0; + UpdateTabContentsSubscription(caller_id, BUBBLE_REMOVED); delete bubble; bubbles_.erase(caller_id); break; diff --git a/chrome/browser/speech/speech_input_bubble_controller.h b/chrome/browser/speech/speech_input_bubble_controller.h index 0a20333..04c92b5 100644 --- a/chrome/browser/speech/speech_input_bubble_controller.h +++ b/chrome/browser/speech/speech_input_bubble_controller.h @@ -11,10 +11,12 @@ #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "chrome/browser/speech/speech_input_bubble.h" +#include "chrome/common/notification_observer.h" namespace gfx { class Rect; } +class NotificationRegistrar; namespace speech_input { @@ -25,7 +27,8 @@ namespace speech_input { // that bubble are reported to the delegate. class SpeechInputBubbleController : public base::RefCountedThreadSafe<SpeechInputBubbleController>, - public SpeechInputBubbleDelegate { + public SpeechInputBubbleDelegate, + public NotificationObserver { public: // All methods of this delegate are called in the IO thread. class Delegate { @@ -73,6 +76,11 @@ class SpeechInputBubbleController virtual void InfoBubbleButtonClicked(SpeechInputBubble::Button button); virtual void InfoBubbleFocusChanged(); + // NotificationObserver implementation. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + private: // The various calls received by this object and handled in the UI thread. enum RequestType { @@ -83,6 +91,11 @@ class SpeechInputBubbleController REQUEST_CLOSE, }; + enum ManageSubscriptionAction { + BUBBLE_ADDED, + BUBBLE_REMOVED + }; + void InvokeDelegateButtonClicked(int caller_id, SpeechInputBubble::Button button); void InvokeDelegateFocusChanged(int caller_id); @@ -91,6 +104,14 @@ class SpeechInputBubbleController const string16& text, float volume); + // Called whenever a bubble was added to or removed from the list. If the + // bubble was being added, this method registers for close notifications with + // the TabContents if this was the first bubble for the tab. Similarly if the + // bubble was being removed, this method unregisters from TabContents if this + // was the last bubble associated with that tab. + void UpdateTabContentsSubscription(int caller_id, + ManageSubscriptionAction action); + // Only accessed in the IO thread. Delegate* delegate_; @@ -102,7 +123,10 @@ class SpeechInputBubbleController // Map of caller-ids to bubble objects. The bubbles are weak pointers owned by // this object and get destroyed by |CloseBubble|. - std::map<int, SpeechInputBubble*> bubbles_; + typedef std::map<int, SpeechInputBubble*> BubbleCallerIdMap; + BubbleCallerIdMap bubbles_; + + scoped_ptr<NotificationRegistrar> registrar_; }; // This typedef is to workaround the issue with certain versions of diff --git a/chrome/browser/speech/speech_input_bubble_controller_unittest.cc b/chrome/browser/speech/speech_input_bubble_controller_unittest.cc index 2ed1f1b..e0ca5ba 100644 --- a/chrome/browser/speech/speech_input_bubble_controller_unittest.cc +++ b/chrome/browser/speech/speech_input_bubble_controller_unittest.cc @@ -5,6 +5,8 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/browser_thread.h" #include "chrome/browser/speech/speech_input_bubble_controller.h" +#include "chrome/test/browser_with_test_window_test.h" +#include "chrome/test/testing_profile.h" #include "gfx/rect.h" #include "testing/gtest/include/gtest/gtest.h" @@ -22,7 +24,10 @@ class MockSpeechInputBubble : public SpeechInputBubbleBase { BUBBLE_TEST_CLICK_TRY_AGAIN, }; - MockSpeechInputBubble(TabContents*, Delegate* delegate, const gfx::Rect&) { + MockSpeechInputBubble(TabContents* tab_contents, + Delegate* delegate, + const gfx::Rect&) + : SpeechInputBubbleBase(tab_contents) { VLOG(1) << "MockSpeechInputBubble created"; MessageLoop::current()->PostTask( FROM_HERE, NewRunnableFunction(&InvokeDelegate, delegate)); @@ -62,12 +67,11 @@ class MockSpeechInputBubble : public SpeechInputBubbleBase { // The test fixture. class SpeechInputBubbleControllerTest : public SpeechInputBubbleControllerDelegate, - public testing::Test { + public BrowserWithTestWindowTest { public: SpeechInputBubbleControllerTest() - : io_loop_(MessageLoop::TYPE_IO), - ui_thread_(BrowserThread::UI), // constructs a new thread and loop - io_thread_(BrowserThread::IO, &io_loop_), // resuses main thread loop + : BrowserWithTestWindowTest(), + io_thread_(BrowserThread::IO), // constructs a new thread and loop cancel_clicked_(false), try_again_clicked_(false), focus_changed_(false), @@ -91,26 +95,28 @@ class SpeechInputBubbleControllerTest } else if (button == SpeechInputBubble::BUTTON_TRY_AGAIN) { try_again_clicked_ = true; } - MessageLoop::current()->Quit(); + message_loop()->PostTask(FROM_HERE, new MessageLoop::QuitTask()); } virtual void InfoBubbleFocusChanged(int caller_id) { VLOG(1) << "Received InfoBubbleFocusChanged"; EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO)); focus_changed_ = true; - MessageLoop::current()->Quit(); + message_loop()->PostTask(FROM_HERE, new MessageLoop::QuitTask()); } // testing::Test methods. virtual void SetUp() { + BrowserWithTestWindowTest::SetUp(); SpeechInputBubble::set_factory( &SpeechInputBubbleControllerTest::CreateBubble); - ui_thread_.Start(); + io_thread_.Start(); } virtual void TearDown() { SpeechInputBubble::set_factory(NULL); - ui_thread_.Stop(); + io_thread_.Stop(); + BrowserWithTestWindowTest::TearDown(); } static void ActivateBubble() { @@ -132,14 +138,19 @@ class SpeechInputBubbleControllerTest // active. MessageLoop::current()->PostTask(FROM_HERE, NewRunnableFunction(&ActivateBubble)); + + // The |tab_contents| parameter would be NULL since the dummy caller id + // passed to CreateBubble would not have matched any active tab. So get a + // real TabContents pointer from the test fixture and pass that, because + // the bubble controller registers for tab close notifications which need + // a valid TabContents. + tab_contents = test_fixture_->browser()->GetSelectedTabContents(); return new MockSpeechInputBubble(tab_contents, delegate, element_rect); } protected: // The main thread of the test is marked as the IO thread and we create a new // one for the UI thread. - MessageLoop io_loop_; - BrowserThread ui_thread_; BrowserThread io_thread_; bool cancel_clicked_; bool try_again_clicked_; diff --git a/chrome/browser/speech/speech_input_bubble_gtk.cc b/chrome/browser/speech/speech_input_bubble_gtk.cc index 2b159bf..eb8b906 100644 --- a/chrome/browser/speech/speech_input_bubble_gtk.cc +++ b/chrome/browser/speech/speech_input_bubble_gtk.cc @@ -54,7 +54,6 @@ class SpeechInputBubbleGtk Delegate* delegate_; InfoBubbleGtk* info_bubble_; - TabContents* tab_contents_; gfx::Rect element_rect_; bool did_invoke_close_; @@ -68,9 +67,9 @@ class SpeechInputBubbleGtk SpeechInputBubbleGtk::SpeechInputBubbleGtk(TabContents* tab_contents, Delegate* delegate, const gfx::Rect& element_rect) - : delegate_(delegate), + : SpeechInputBubbleBase(tab_contents), + delegate_(delegate), info_bubble_(NULL), - tab_contents_(tab_contents), element_rect_(element_rect), did_invoke_close_(false), label_(NULL), @@ -145,10 +144,10 @@ void SpeechInputBubbleGtk::Show() { gtk_container_add(GTK_CONTAINER(content), vbox); GtkThemeProvider* theme_provider = GtkThemeProvider::GetFrom( - tab_contents_->profile()); + tab_contents()->profile()); gfx::Rect rect(element_rect_.x() + kBubbleTargetOffsetX, element_rect_.y() + element_rect_.height(), 1, 1); - info_bubble_ = InfoBubbleGtk::Show(tab_contents_->GetNativeView(), + info_bubble_ = InfoBubbleGtk::Show(tab_contents()->GetNativeView(), &rect, content, InfoBubbleGtk::ARROW_LOCATION_TOP_LEFT, diff --git a/chrome/browser/speech/speech_input_bubble_mac.mm b/chrome/browser/speech/speech_input_bubble_mac.mm index d4a54f2..912370e 100644 --- a/chrome/browser/speech/speech_input_bubble_mac.mm +++ b/chrome/browser/speech/speech_input_bubble_mac.mm @@ -30,7 +30,6 @@ class SpeechInputBubbleImpl : public SpeechInputBubbleBase { private: scoped_nsobject<SpeechInputWindowController> window_; - TabContents* tab_contents_; Delegate* delegate_; gfx::Rect element_rect_; }; @@ -38,7 +37,7 @@ class SpeechInputBubbleImpl : public SpeechInputBubbleBase { SpeechInputBubbleImpl::SpeechInputBubbleImpl(TabContents* tab_contents, Delegate* delegate, const gfx::Rect& element_rect) - : tab_contents_(tab_contents), + : SpeechInputBubbleBase(tab_contents), delegate_(delegate), element_rect_(element_rect) { } @@ -62,7 +61,7 @@ void SpeechInputBubbleImpl::Show() { // Find the screen coordinates for the given tab and position the bubble's // arrow anchor point inside that to point at the bottom-left of the html // input element rect. - gfx::NativeView view = tab_contents_->view()->GetNativeView(); + gfx::NativeView view = tab_contents()->view()->GetNativeView(); NSRect tab_bounds = [view bounds]; NSPoint anchor = NSMakePoint( tab_bounds.origin.x + element_rect_.x() + kBubbleTargetOffsetX, @@ -72,7 +71,7 @@ void SpeechInputBubbleImpl::Show() { anchor = [[view window] convertBaseToScreen:anchor]; window_.reset([[SpeechInputWindowController alloc] - initWithParentWindow:tab_contents_->view()->GetTopLevelNativeWindow() + initWithParentWindow:tab_contents()->view()->GetTopLevelNativeWindow() delegate:delegate_ anchoredAt:anchor]); diff --git a/chrome/browser/speech/speech_input_bubble_views.cc b/chrome/browser/speech/speech_input_bubble_views.cc index 3027da3..2140ef4 100644 --- a/chrome/browser/speech/speech_input_bubble_views.cc +++ b/chrome/browser/speech/speech_input_bubble_views.cc @@ -13,10 +13,6 @@ #include "chrome/browser/tab_contents/tab_contents_view.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/views/info_bubble.h" -#include "chrome/common/notification_observer.h" -#include "chrome/common/notification_registrar.h" -#include "chrome/common/notification_source.h" -#include "chrome/common/notification_type.h" #include "gfx/canvas.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" @@ -200,8 +196,7 @@ void ContentView::Layout() { // Implementation of SpeechInputBubble. class SpeechInputBubbleImpl : public SpeechInputBubbleBase, - public InfoBubbleDelegate, - public NotificationObserver { + public InfoBubbleDelegate { public: SpeechInputBubbleImpl(TabContents* tab_contents, Delegate* delegate, @@ -220,11 +215,6 @@ class SpeechInputBubbleImpl // |element_rect| is the html element's bounds in page coordinates. gfx::Rect GetInfoBubbleTarget(const gfx::Rect& element_rect); - // NotificationObserver implementation. - virtual void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - // InfoBubbleDelegate virtual void InfoBubbleClosing(InfoBubble* info_bubble, bool closed_by_escape); @@ -234,9 +224,7 @@ class SpeechInputBubbleImpl private: Delegate* delegate_; InfoBubble* info_bubble_; - TabContents* tab_contents_; ContentView* bubble_content_; - NotificationRegistrar registrar_; gfx::Rect element_rect_; // Set to true if the object is being destroyed normally instead of the @@ -249,9 +237,9 @@ class SpeechInputBubbleImpl SpeechInputBubbleImpl::SpeechInputBubbleImpl(TabContents* tab_contents, Delegate* delegate, const gfx::Rect& element_rect) - : delegate_(delegate), + : SpeechInputBubbleBase(tab_contents), + delegate_(delegate), info_bubble_(NULL), - tab_contents_(tab_contents), bubble_content_(NULL), element_rect_(element_rect), did_invoke_close_(false) { @@ -265,26 +253,14 @@ SpeechInputBubbleImpl::~SpeechInputBubbleImpl() { gfx::Rect SpeechInputBubbleImpl::GetInfoBubbleTarget( const gfx::Rect& element_rect) { gfx::Rect container_rect; - tab_contents_->GetContainerBounds(&container_rect); + tab_contents()->GetContainerBounds(&container_rect); return gfx::Rect( container_rect.x() + element_rect.x() + kBubbleTargetOffsetX, container_rect.y() + element_rect.y() + element_rect.height(), 1, 1); } -void SpeechInputBubbleImpl::Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details) { - if (type == NotificationType::TAB_CONTENTS_DESTROYED) { - delegate_->InfoBubbleButtonClicked(SpeechInputBubble::BUTTON_CANCEL); - } else { - NOTREACHED() << "Unknown notification"; - } -} - void SpeechInputBubbleImpl::InfoBubbleClosing(InfoBubble* info_bubble, bool closed_by_escape) { - registrar_.Remove(this, NotificationType::TAB_CONTENTS_DESTROYED, - Source<TabContents>(tab_contents_)); info_bubble_ = NULL; bubble_content_ = NULL; if (!did_invoke_close_) @@ -307,7 +283,7 @@ void SpeechInputBubbleImpl::Show() { UpdateLayout(); views::Widget* parent = views::Widget::GetWidgetFromNativeWindow( - tab_contents_->view()->GetTopLevelNativeWindow()); + tab_contents()->view()->GetTopLevelNativeWindow()); info_bubble_ = InfoBubble::Show(parent, GetInfoBubbleTarget(element_rect_), BubbleBorder::TOP_LEFT, bubble_content_, @@ -319,9 +295,6 @@ void SpeechInputBubbleImpl::Show() { // to end so the caller can manage this object's life cycle like a normal // stack based or member variable object. info_bubble_->set_fade_away_on_close(false); - - registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED, - Source<TabContents>(tab_contents_)); } void SpeechInputBubbleImpl::Hide() { diff --git a/chrome/browser/speech/speech_input_manager.cc b/chrome/browser/speech/speech_input_manager.cc index bb63b34..25f0550 100644 --- a/chrome/browser/speech/speech_input_manager.cc +++ b/chrome/browser/speech/speech_input_manager.cc @@ -139,7 +139,6 @@ class SpeechInputManagerImpl : public SpeechInputManager, // Starts/restarts recognition for an existing request. void StartRecognitionForRequest(int caller_id); - SpeechInputManagerDelegate* delegate_; typedef std::map<int, SpeechInputRequest> SpeechRecognizerMap; SpeechRecognizerMap requests_; int recording_caller_id_; |