assistant: remove response process v2 flags

Clean up launched feature flag.

Bug: None
Change-Id: I205696223fd6db4136220f145c5237273df07037
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2590709Reviewed-by: Tao Wu <wutao@chromium.org>
Commit-Queue: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837737}

ash/assistant/assistant_interaction_controller_impl.h

@@ -130,15 +130,8 @@ class AssistantInteractionControllerImpl
 
  private:
   void OnTabletModeChanged();
-
-  bool HasUnprocessedPendingResponse();
   bool HasActiveInteraction() const;
-
-  void OnProcessPendingResponse();
-  void OnPendingResponseProcessed(bool is_completed);
-
   void OnUiVisible(AssistantEntryPoint entry_point);
-
   void StartScreenContextInteraction(bool include_assistant_structure,
                                      const gfx::Rect& region,
                                      AssistantQuerySource query_source);

ash/assistant/assistant_interaction_controller_impl_unittest.cc

@@ -73,19 +73,9 @@ class AssistantInteractionSubscriberMock
 
 // AssistantInteractionControllerImplTest --------------------------------------
 
-class AssistantInteractionControllerImplTest
-    : public AssistantAshTestBase,
-      public testing::WithParamInterface<bool> {
+class AssistantInteractionControllerImplTest : public AssistantAshTestBase {
  public:
-  AssistantInteractionControllerImplTest() {
-    if (GetParam()) {
-      feature_list_.InitAndEnableFeature(
-          chromeos::assistant::features::kAssistantResponseProcessingV2);
-    } else {
-      feature_list_.InitAndDisableFeature(
-          chromeos::assistant::features::kAssistantResponseProcessingV2);
-    }
-  }
+  AssistantInteractionControllerImplTest() = default;
 
   AssistantInteractionControllerImpl* interaction_controller() {
     return static_cast<AssistantInteractionControllerImpl*>(
@@ -111,14 +101,11 @@ class AssistantInteractionControllerImplTest
     result.localized_app_name = app_name;
     return result;
   }
-
- private:
-  base::test::ScopedFeatureList feature_list_;
 };
 
 }  // namespace
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldBecomeActiveWhenInteractionStarts) {
   EXPECT_EQ(interaction_model()->interaction_state(),
             InteractionState::kInactive);
@@ -130,7 +117,7 @@ TEST_P(AssistantInteractionControllerImplTest,
             InteractionState::kActive);
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldReturnErrorWhenOpenAppIsCalledWhileInactive) {
   EXPECT_EQ(interaction_model()->interaction_state(),
             InteractionState::kInactive);
@@ -140,7 +127,7 @@ TEST_P(AssistantInteractionControllerImplTest,
   EXPECT_EQ(result, kErrorResult);
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldReturnErrorWhenOpenAppIsCalledWithoutAnAndroidIntentHelper) {
   StartInteraction();
 
@@ -149,7 +136,7 @@ TEST_P(AssistantInteractionControllerImplTest,
   EXPECT_EQ(result, kErrorResult);
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldReturnErrorWhenOpenAppIsCalledForUnknownAndroidApp) {
   StartInteraction();
   FakeAndroidIntentHelper fake_helper;
@@ -157,7 +144,7 @@ TEST_P(AssistantInteractionControllerImplTest,
                               CreateAndroidAppInfo("unknown-app-name")));
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldLaunchAppAndReturnSuccessWhenOpenAppIsCalled) {
   const std::string app_name = "AppName";
   const std::string intent = "intent://AppName";
@@ -172,7 +159,7 @@ TEST_P(AssistantInteractionControllerImplTest,
   EXPECT_EQ(intent, fake_helper.last_launched_android_intent());
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldAddSchemeToIntentWhenLaunchingAndroidApp) {
   const std::string app_name = "AppName";
   const std::string intent = "#Intent-without-a-scheme";
@@ -188,7 +175,7 @@ TEST_P(AssistantInteractionControllerImplTest,
   EXPECT_EQ(intent_with_scheme, fake_helper.last_launched_android_intent());
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldCorrectlyMapSuggestionTypeToQuerySource) {
   // Mock Assistant interaction subscriber.
   StrictMock<AssistantInteractionSubscriberMock> mock(assistant_service());
@@ -226,7 +213,7 @@ TEST_P(AssistantInteractionControllerImplTest,
   }
 }
 
-TEST_P(AssistantInteractionControllerImplTest, ShouldDisplayGenericErrorOnce) {
+TEST_F(AssistantInteractionControllerImplTest, ShouldDisplayGenericErrorOnce) {
   StartInteraction();
 
   // Call OnTtsStarted twice to mimic the behavior of libassistant when network
@@ -253,7 +240,7 @@ TEST_P(AssistantInteractionControllerImplTest, ShouldDisplayGenericErrorOnce) {
   EXPECT_EQ(ui_elements.front()->type(), AssistantUiElementType::kError);
 }
 
-TEST_P(AssistantInteractionControllerImplTest,
+TEST_F(AssistantInteractionControllerImplTest,
        ShouldUpdateTimeOfLastInteraction) {
   MockAssistantInteractionSubscriber mock_subscriber;
   ScopedAssistantInteractionSubscriber scoped_subscriber{&mock_subscriber};
@@ -276,10 +263,4 @@ TEST_P(AssistantInteractionControllerImplTest,
 
   EXPECT_NEAR(actual.InSeconds(), expected.InSeconds(), 1);
 }
-
-// We parameterize all AssistantInteractionControllerImplTests to verify that
-// they work for both response processing v1 as well as response processing v2.
-INSTANTIATE_TEST_SUITE_P(All,
-                         AssistantInteractionControllerImplTest,
-                         testing::Bool());
 }  // namespace ash

ash/assistant/model/assistant_response.cc

@@ -124,13 +124,6 @@ void AssistantResponse::RemoveObserver(
 
 void AssistantResponse::AddUiElement(
     std::unique_ptr<AssistantUiElement> ui_element) {
-  // In processing v1, UI elements are immediately added to the response.
-  if (!chromeos::assistant::features::IsResponseProcessingV2Enabled()) {
-    ui_elements_.push_back(std::move(ui_element));
-    NotifyUiElementAdded(ui_elements_.back().get());
-    return;
-  }
-
   // In processing v2, UI elements are first cached in a pending state...
   auto pending_ui_element = std::make_unique<PendingUiElement>();
   pending_ui_element->ui_element = std::move(ui_element);

ash/assistant/ui/main_stage/animated_container_view.cc

@@ -18,12 +18,6 @@
 
 namespace ash {
 
-namespace {
-
-using chromeos::assistant::features::IsResponseProcessingV2Enabled;
-
-}  // namespace
-
 // AnimatedContainerView::ScopedDisablePreferredSizeChanged --------------------
 
 class AnimatedContainerView::ScopedDisablePreferredSizeChanged {
@@ -53,7 +47,7 @@ AnimatedContainerView::AnimatedContainerView(AssistantViewDelegate* delegate)
 }
 
 AnimatedContainerView::~AnimatedContainerView() {
-  if (IsResponseProcessingV2Enabled() && response_)
+  if (response_)
     response_.get()->RemoveObserver(this);
 
   if (AssistantInteractionController::Get())
@@ -120,7 +114,7 @@ void AnimatedContainerView::OnSuggestionsAdded(
 }
 
 void AnimatedContainerView::RemoveAllViews() {
-  if (IsResponseProcessingV2Enabled() && response_)
+  if (response_)
     response_.get()->RemoveObserver(this);
 
   // We explicitly abort all in progress animations here because we will remove
@@ -170,7 +164,7 @@ std::unique_ptr<ElementAnimator> AnimatedContainerView::HandleSuggestion(
 
 void AnimatedContainerView::ChangeResponse(
     const scoped_refptr<const AssistantResponse>& response) {
-  if (IsResponseProcessingV2Enabled() && response_)
+  if (response_)
     response_.get()->RemoveObserver(this);
 
   // We may have to postpone the response while we animate the previous response
@@ -210,13 +204,6 @@ void AnimatedContainerView::ChangeResponse(
 
 void AnimatedContainerView::AddResponse(
     scoped_refptr<const AssistantResponse> response) {
-  if (!IsResponseProcessingV2Enabled()) {
-    // The response should be fully processed before it is presented.
-    // Note that ProcessingState is only used in v1 of response processing.
-    DCHECK_EQ(AssistantResponse::ProcessingState::kProcessed,
-              response->processing_state());
-  }
-
   // All children should be animated out and removed before the new response is
   // added.
   DCHECK(content_view()->children().empty());
@@ -225,11 +212,9 @@ void AnimatedContainerView::AddResponse(
   // destroyed before we have removed associated views from the view hierarchy.
   response_ = std::move(response);
 
-  if (IsResponseProcessingV2Enabled()) {
-    // In response processing v2, we observe the |response_| so that we handle
-    // new suggestions and UI elements that continue to stream in.
-    response_.get()->AddObserver(this);
-  }
+  // In response processing v2, we observe the |response_| so that we handle
+  // new suggestions and UI elements that continue to stream in.
+  response_.get()->AddObserver(this);
 
   // We can prevent over-propagation of the PreferredSizeChanged event by
   // stopping propagation during batched view hierarchy add/remove operations.

chromeos/services/assistant/proxy/service_controller_proxy.cc

@@ -44,10 +44,8 @@ void FillServerExperimentIds(std::vector<std::string>* server_experiment_ids) {
   if (base::FeatureList::IsEnabled(features::kAssistantAppSupport))
     server_experiment_ids->emplace_back(kServersideOpenAppExperimentId);
 
-  if (features::IsResponseProcessingV2Enabled()) {
-    server_experiment_ids->emplace_back(
-        kServersideResponseProcessingV2ExperimentId);
-  }
+  server_experiment_ids->emplace_back(
+      kServersideResponseProcessingV2ExperimentId);
 }
 
 void SetServerExperiments(

chromeos/services/assistant/public/cpp/features.cc

@@ -29,9 +29,6 @@ const base::Feature kAssistantDebugging{"AssistantDebugging",
 const base::Feature kAssistantLauncherChipIntegration{
     "AssistantLauncherChipIntegration", base::FEATURE_DISABLED_BY_DEFAULT};
 
-const base::Feature kAssistantResponseProcessingV2{
-    "AssistantResponseProcessingV2", base::FEATURE_ENABLED_BY_DEFAULT};
-
 const base::Feature kAssistantRoutines{"AssistantRoutines",
                                        base::FEATURE_DISABLED_BY_DEFAULT};
 
@@ -124,10 +121,6 @@ bool IsLibAssistantBetaBackendEnabled() {
   return base::FeatureList::IsEnabled(kEnableLibAssistantBetaBackend);
 }
 
-bool IsResponseProcessingV2Enabled() {
-  return base::FeatureList::IsEnabled(kAssistantResponseProcessingV2);
-}
-
 bool IsRoutinesEnabled() {
   return base::FeatureList::IsEnabled(kAssistantRoutines);
 }
@@ -147,9 +140,7 @@ bool IsVoiceMatchDisabled() {
 }
 
 bool IsWaitSchedulingEnabled() {
-  // Wait scheduling is only supported for response processing v2 and routines.
-  return base::FeatureList::IsEnabled(kAssistantWaitScheduling) &&
-         (IsResponseProcessingV2Enabled() || IsRoutinesEnabled());
+  return base::FeatureList::IsEnabled(kAssistantWaitScheduling);
 }
 
 }  // namespace features

chromeos/services/assistant/public/cpp/features.h

@@ -32,12 +32,6 @@ extern const base::Feature kAssistantBetterOnboarding;
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
 extern const base::Feature kAssistantLauncherChipIntegration;
 
-// When enabled, Assistant will use response processing V2. This is a set of
-// synced client and server changes which will turn on default parallel client
-// op processing and eager (streaming) UI element rendering.
-COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
-extern const base::Feature kAssistantResponseProcessingV2;
-
 // Enables Assistant routines.
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
 extern const base::Feature kAssistantRoutines;