Quick Answers: Fix possible timing issue

Adding state guard to fix possible timing issue in
QuickAnswersMenuObserver and QuickAnswersControllerImpl.

Bug: b/163112381
Test: Manual Test
Change-Id: Ibde6a0c3d643d915ad5e2836e10ba45bae501ff9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342140
Auto-Submit: Yue Li <updowndota@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796924}

ash/quick_answers/quick_answers_controller_impl.cc

@@ -70,6 +70,8 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
   if (!is_eligible_)
     return;
 
+  is_session_active_ = true;
+
   // Cache anchor-bounds and query.
   anchor_bounds_ = anchor_bounds;
   // Initially, title is same as query. Title and query can be overridden based
@@ -94,6 +96,7 @@ void QuickAnswersControllerImpl::MaybeShowQuickAnswers(
 }
 
 void QuickAnswersControllerImpl::DismissQuickAnswers(bool is_active) {
+  is_session_active_ = false;
   MaybeDismissQuickAnswersConsent();
   bool closed = quick_answers_ui_controller_->CloseQuickAnswersView();
   quick_answers_client_->OnQuickAnswersDismissed(
@@ -153,6 +156,9 @@ void QuickAnswersControllerImpl::OnRequestPreprocessFinished(
     return;
   }
 
+  if (!is_session_active_)
+    return;
+
   query_ = processed_request.preprocessed_output.query;
   title_ = processed_request.preprocessed_output.intent_text;
 
@@ -194,7 +200,8 @@ void QuickAnswersControllerImpl::OnUserConsentGranted() {
       chromeos::quick_answers::ConsentInteractionType::kAccept);
 
   // Display Quick-Answer for the cached query when user consents.
-  MaybeShowQuickAnswers(anchor_bounds_, title_, context_);
+  if (is_session_active_)
+    MaybeShowQuickAnswers(anchor_bounds_, title_, context_);
 }
 
 void QuickAnswersControllerImpl::OnConsentSettingsRequestedByUser() {

ash/quick_answers/quick_answers_controller_impl.h

@@ -118,6 +118,9 @@ class ASH_EXPORT QuickAnswersControllerImpl
 
   // The last received QuickAnswer from client.
   std::unique_ptr<chromeos::quick_answers::QuickAnswer> quick_answer_;
+
+  // If currently there is an active quick answers session.
+  bool is_session_active_ = false;
 };
 
 }  // namespace ash

chrome/browser/renderer_context_menu/quick_answers_menu_observer.cc

@@ -134,6 +134,8 @@ void QuickAnswersMenuObserver::InitMenu(
 void QuickAnswersMenuObserver::OnContextMenuShown(
     const content::ContextMenuParams& params,
     const gfx::Rect& bounds_in_screen) {
+  is_context_menu_showing_ = true;
+
   if (!IsRichUiEnabled())
     return;
 
@@ -163,12 +165,14 @@ void QuickAnswersMenuObserver::OnContextMenuShown(
 void QuickAnswersMenuObserver::OnContextMenuViewBoundsChanged(
     const gfx::Rect& bounds_in_screen) {
   bounds_in_screen_ = bounds_in_screen;
-  if (!quick_answers_controller_)
+  if (!quick_answers_controller_ || !is_context_menu_showing_)
     return;
   quick_answers_controller_->UpdateQuickAnswersAnchorBounds(bounds_in_screen);
 }
 
 void QuickAnswersMenuObserver::OnMenuClosed() {
+  is_context_menu_showing_ = false;
+
   if (!IsRichUiEnabled())
     return;
 
@@ -268,6 +272,9 @@ void QuickAnswersMenuObserver::OnTextSurroundingSelectionAvailable(
     const base::string16& surrounding_text,
     uint32_t start_offset,
     uint32_t end_offset) {
+  if (!is_context_menu_showing_)
+    return;
+
   Context context;
   context.surrounding_text = base::UTF16ToUTF8(surrounding_text);
   context.device_properties.language = GetDeviceLanguage();

chrome/browser/renderer_context_menu/quick_answers_menu_observer.h

@@ -84,6 +84,11 @@ class QuickAnswersMenuObserver
   // Whether commands other than quick answers is executed.
   bool is_other_command_executed_ = false;
 
+  // Whether the context menu is currently showing.
+  // TODO(updowndota): Instead of adding flags to check the callback, add a
+  // global quick answers state in the client to guard the sequence.
+  bool is_context_menu_showing_ = false;
+
   base::WeakPtrFactory<QuickAnswersMenuObserver> weak_factory_{this};
 };