New issue
Advanced search Search tips

Issue 739097 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task



Sign in to add a comment

Move ArcSessionManager to components/arc.

Project Member Reported by hidehiko@chromium.org, Jul 4 2017

Issue description

Maybe better to have another tracking issue for this topic explicitly.

Now ArcSessionManager is very complex class.
We probably should move ArcSessionManager to components/arc with refactoring in a manageable form with precise tests.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 12 2017

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

commit e2dce251e5f9735409dbcda860ba18728c280ed2
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Wed Jul 12 04:22:13 2017

Remove LocalActivityResolver.

Now ArcIntentHelperBridge can be obtained via ArcServiceManager.
So, LocalActivityResolver can be merged into ArcServiceManager
to get rid of unnecessary reference from ArcServiceManager to it.

BUG=739097
TEST=Ran trybot.

Change-Id: Idaee93706e14b268feb28d98811f7b290de2165e
Reviewed-on: https://chromium-review.googlesource.com/566785
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485858}
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/chrome/browser/chromeos/arc/arc_service_launcher.cc
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.cc
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/components/arc/BUILD.gn
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/components/arc/arc_service_manager.cc
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/components/arc/arc_service_manager.h
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/components/arc/intent_helper/arc_intent_helper_bridge.cc
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/components/arc/intent_helper/arc_intent_helper_bridge.h
[modify] https://crrev.com/e2dce251e5f9735409dbcda860ba18728c280ed2/components/arc/intent_helper/arc_intent_helper_bridge_unittest.cc
[delete] https://crrev.com/27d2945c584403c55da1c1692c36568d5718f971/components/arc/intent_helper/local_activity_resolver.cc
[delete] https://crrev.com/27d2945c584403c55da1c1692c36568d5718f971/components/arc/intent_helper/local_activity_resolver.h
[delete] https://crrev.com/27d2945c584403c55da1c1692c36568d5718f971/components/arc/intent_helper/local_activity_resolver_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 14 2017

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

commit e665e3f72f2c55b812f37284ef1425b69aa487fc
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri Jul 14 15:05:34 2017

Introduce ArcBrowserContextKeyedServiceFactoryBase.

While migrating ArcService into BrowserContextKeyedService,
it turned out the boilerplate looks much bigger than what we expected.

To minimize it reasonably, this CL introduces
ArcBrowserContextKeyedServiceFactoryBase and use it for
already migrated service classes.

BUG=739097
TEST=Ran trybot. Ran on DUT.

Change-Id: I13a1e7019c06ea64fa48cec6ecf7a9e589e84b00
Reviewed-on: https://chromium-review.googlesource.com/569847
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486759}
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.cc
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge.h
[delete] https://crrev.com/b8dacd14bdc12e1c8c895818d866e7892f15f96e/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_factory.cc
[delete] https://crrev.com/b8dacd14bdc12e1c8c895818d866e7892f15f96e/chrome/browser/chromeos/arc/accessibility/arc_accessibility_helper_bridge_factory.h
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/arc/arc_service_launcher.cc
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/arc/auth/arc_auth_service.cc
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/arc/auth/arc_auth_service.h
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc
[delete] https://crrev.com/b8dacd14bdc12e1c8c895818d866e7892f15f96e/chrome/browser/chromeos/arc/auth/arc_auth_service_factory.cc
[delete] https://crrev.com/b8dacd14bdc12e1c8c895818d866e7892f15f96e/chrome/browser/chromeos/arc/auth/arc_auth_service_factory.h
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/components/arc/BUILD.gn
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/components/arc/DEPS
[add] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/components/arc/arc_browser_context_keyed_service_factory_base.h
[modify] https://crrev.com/e665e3f72f2c55b812f37284ef1425b69aa487fc/components/arc/arc_service_manager.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 13 2017

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

commit 33e9adfd064d2af4085944599372440e170c5b40
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Wed Sep 13 06:12:23 2017

Move ARC related prefs to components/arc/arc_prefs.

This is preparation to move some code from
chrome/browser/chromeos/arc to components/arc,
so that those moved code can refer prefs.

BUG=739097
TEST=Ran trybot.

Change-Id: I0f52e80a88c7d34b7833d86e4db97cd94eb253ba
Reviewed-on: https://chromium-review.googlesource.com/642640
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Bartosz Fabianowski <bartfab@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501553}
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/app_mode/arc/arc_kiosk_app_service.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_migration_guide_notification.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_play_store_enabled_preference_handler_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_session_manager.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_session_manager.h
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_session_manager_browsertest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_session_manager_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_util.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/arc_util_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/auth/arc_auth_service.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/intent_helper/arc_settings_service.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/intent_helper/arc_settings_service_browsertest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/notification/arc_provision_notification_service_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/optin/arc_optin_preference_handler.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/optin/arc_terms_of_service_default_negotiator_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/policy/arc_policy_bridge.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/policy/arc_policy_bridge.h
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/policy/arc_policy_bridge_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_arc_home_service.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/arc/voice_interaction/arc_voice_interaction_framework_service.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/first_run/first_run.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/login/wizard_controller.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/chromeos/note_taking_helper_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/extensions/api/settings_private/prefs_util.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/policy/default_geolocation_policy_handler.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/policy/default_geolocation_policy_handler_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/app_list/arc/arc_package_sync_data_type_controller.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/app_list/arc/arc_pai_starter.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/browser/ui/webui/chromeos/login/arc_terms_of_service_screen_handler.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/common/pref_names.cc
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/chrome/common/pref_names.h
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/components/arc/BUILD.gn
[modify] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/components/arc/DEPS
[add] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/components/arc/arc_export.h
[add] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/components/arc/arc_prefs.cc
[add] https://crrev.com/33e9adfd064d2af4085944599372440e170c5b40/components/arc/arc_prefs.h

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 22 2017

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

commit c17522b543fb042d5accdc0f9faca96151bdfcd2
Author: Hidehiko Abe <hidehiko@chromium.org>
Date: Fri Sep 22 17:46:07 2017

Extract ArcDataRemover.

This CL extracts an operation to remove ARC user data
directory from ArcSessionManager to ArcDataRemover.

BUG=739097
TEST=Ran trybot.
TEST=Ran on DUT and made sure session_manager removes the data directory.

Change-Id: I1898c15b68475595d9fcd937599c41c061194976
Reviewed-on: https://chromium-review.googlesource.com/675143
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503788}
[modify] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/chrome/browser/chromeos/arc/arc_session_manager.cc
[modify] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/chrome/browser/chromeos/arc/arc_session_manager.h
[modify] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/components/arc/BUILD.gn
[add] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/components/arc/arc_data_remover.cc
[add] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/components/arc/arc_data_remover.h
[add] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/components/arc/arc_data_remover_unittest.cc
[modify] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/components/arc/arc_prefs.cc
[modify] https://crrev.com/c17522b543fb042d5accdc0f9faca96151bdfcd2/components/arc/arc_prefs.h

Cc: -lhchavez@chromium.org

Sign in to add a comment