New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758650 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 26 days ago
Closed: Nov 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Move voice interaction specific methods out of Shell and ShellObserver

Project Member Reported by kaznacheev@chromium.org, Aug 24 2017

Issue description

There already are two pairs of Notify*/On* methods, and the third one is about to be added.

These need to move to a separate controller and observer.
 
Labels: m61debt
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/39d115f8066cec837318326d6d0fc3752e96ef05

commit 39d115f8066cec837318326d6d0fc3752e96ef05
Author: Vladislav Kaznacheev <kaznacheev@google.com>
Date: Fri Aug 25 17:19:46 2017

Comment on technical debt recently created in ash::Shell

Voice interaction related methods should move out of Shell
and ShellObserver soon

TBR=jamescook@chromium.org

Bug:  758650 
Change-Id: I13c1737e498b2d1f04b019e32c8a33ffda4cbcbd
Reviewed-on: https://chromium-review.googlesource.com/635947
Reviewed-by: Vladislav Kaznacheev <kaznacheev@chromium.org>
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497446}
[modify] https://crrev.com/39d115f8066cec837318326d6d0fc3752e96ef05/ash/shell.h
[modify] https://crrev.com/39d115f8066cec837318326d6d0fc3752e96ef05/ash/shell_observer.h

TODO reminder for my part:
- Move the cached voice interaction state and flags into the controller
- Rename the methods name to be consist with the flag name
- Switch to use SessionController::IsUserPrimary() in app_list_button
Cc: -updowndota@chromium.org kaznacheev@chromium.org
Owner: updowndota@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa

commit a33960a0caeb87e84be0a8e60ebfccbd24f25aaa
Author: Yue Li <updowndota@chromium.org>
Date: Wed Nov 01 22:06:07 2017

Refactor voice interaction related Shell methods and Shell observers

- Add VoiceInteractionController and VoiceInteractionObserver,
  remove the corresponding Shell methods.

- Add VoiceInteractionControllerClient, notifies the controller
  through mojo calls.

- Refactor ArcVIFrameworkService to use the client instead of calling
  Shell.

- Add unittests for VoiceInteractionController, added test cases in
  framework service unittest for VoiceInteractionControllerClient.

- Update usages of the refactored methods(palette_tray_unittest,
  metalayer_mode, app_list_button).

Bug:  757012 ,  758650 
Change-Id: I6b5ff3e0164bb0ebbf7838b79e242533fd863b3b
Reviewed-on: https://chromium-review.googlesource.com/734126
Commit-Queue: Yue Li <updowndota@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513295}
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/BUILD.gn
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/mojo_interface_factory.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/mus/manifest.json
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/mus/standalone/manifest.json
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/public/cpp/BUILD.gn
[delete] https://crrev.com/70febf8c7f665b5c4a9051b589981bf37db7ef7e/ash/public/cpp/voice_interaction_state.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/public/interfaces/BUILD.gn
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/public/interfaces/voice_interaction_controller.mojom
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/shelf/app_list_button.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/shelf/app_list_button.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/shelf/app_list_button_unittest.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/shell.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/shell.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/shell_observer.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/system/palette/palette_tray_unittest.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/system/palette/tools/metalayer_mode.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/system/palette/tools/metalayer_mode.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/system/palette/tools/metalayer_unittest.cc
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/voice_interaction/OWNERS
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/voice_interaction/voice_interaction_controller.cc
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/voice_interaction/voice_interaction_controller.h
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/voice_interaction/voice_interaction_controller_unittest.cc
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/ash/voice_interaction/voice_interaction_observer.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service_unittest.cc
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.cc
[add] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/components/arc/BUILD.gn
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/components/arc/common/typemaps.gni
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/components/arc/common/voice_interaction_framework.mojom
[delete] https://crrev.com/70febf8c7f665b5c4a9051b589981bf37db7ef7e/components/arc/common/voice_interaction_framework.typemap
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/components/arc/test/fake_voice_interaction_framework_instance.cc
[modify] https://crrev.com/a33960a0caeb87e84be0a8e60ebfccbd24f25aaa/components/arc/test/fake_voice_interaction_framework_instance.h
[delete] https://crrev.com/70febf8c7f665b5c4a9051b589981bf37db7ef7e/components/arc/voice_interaction/OWNERS
[delete] https://crrev.com/70febf8c7f665b5c4a9051b589981bf37db7ef7e/components/arc/voice_interaction/voice_interaction_struct_traits.h

Status: Fixed (was: Assigned)

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 8 by dchan@chromium.org, Jan 23 2018

Status: Fixed (was: Archived)

Sign in to add a comment