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

Issue 626695 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Last visit 16 days ago
Closed: Jul 2016
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

ARC: Improve ArcBridgeService interface

Project Member Reported by lhchavez@chromium.org, Jul 8 2016

Issue description

ArcBridgeService has tons of boilerplate that can be removed. It also makes testing a bit awkward since it has to jump through hoops to ensure all Mojo messages are delivered, while they could be run in a single thread instead.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 13 2016

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

commit de2de961f2851a852febfb7223b0f27df5e6afb9
Author: lhchavez <lhchavez@chromium.org>
Date: Wed Jul 13 04:43:01 2016

arc: Revamp the ArcBridgeService interface

Adding a new host/instance pair required a lot of boilerplate. It also
made it very hard to properly do fakes for tests. This change adds two
more interfaces: InterfaceHolder<T> and InterfaceObserver<T>, so that
ArcBridgeService can stop having hundreds of methods to deal with
instance lifecycles.

BUG= 626695 
TEST=trybots

Review-Url: https://codereview.chromium.org/2133503002
Cr-Commit-Position: refs/heads/master@{#405025}

[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_downloads_watcher_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_navigation_throttle.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_policy_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_policy_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_policy_bridge_unittest.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_process_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_process_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_settings_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_settings_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/gpu_arc_video_service_host.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/gpu_arc_video_service_host.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/file_manager/arc_file_tasks.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/memory/tab_manager_delegate_chromeos.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/memory/tab_manager_delegate_chromeos.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/task_management/providers/arc/arc_process_task.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/task_management/providers/arc/arc_process_task.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/app_list/arc/arc_app_utils.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc.gypi
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/BUILD.gn
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/arc_bridge_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/audio/arc_audio_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/audio/arc_audio_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/bluetooth/arc_bluetooth_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/bluetooth/arc_bluetooth_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/clipboard/arc_clipboard_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/clipboard/arc_clipboard_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/crash_collector/arc_crash_collector_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/crash_collector/arc_crash_collector_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/ime/arc_ime_bridge_impl.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/ime/arc_ime_bridge_impl.h
[add] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/instance_holder.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/activity_icon_loader.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/arc_intent_helper_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/arc_intent_helper_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/link_handler_model_impl.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/metrics/arc_metrics_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/metrics/arc_metrics_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/net/arc_net_host_impl.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/net/arc_net_host_impl.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/obb_mounter/arc_obb_mounter_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/obb_mounter/arc_obb_mounter_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/power/arc_power_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/power/arc_power_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/storage_manager/arc_storage_manager.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/storage_manager/arc_storage_manager.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/test/fake_policy_instance.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/test/fake_policy_instance.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/window_manager/arc_window_manager_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/window_manager/arc_window_manager_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/BUILD.gn
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/notification/arc_notification_manager.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/notification/arc_notification_manager_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13 2016

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

commit c5fffc64c14a0464f8870bbe91df7e945f42c3ca
Author: lhchavez <lhchavez@chromium.org>
Date: Wed Jul 13 14:01:41 2016

arc: Use the new InstanceHolder for unittests

Instead of having unit tests go through Mojo and jump through hoops to
ensure that all messages have propagated to all the threads, we can use
the new arc::ArcBridgeService::InstanceHolder<T>::SetInstance() to
directly set the concrete fake (and optional version). This simplifies
testing quite a bit.

BUG= 626695 
TEST=trybots

Review-Url: https://codereview.chromium.org/2138513002
Cr-Commit-Position: refs/heads/master@{#405138}

[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service_impl.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service_impl.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/ui/arc/notification/arc_notification_manager_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 13 2016

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

commit 03b1e37434a736bbb5c764dd980787e54c93868b
Author: engedy <engedy@chromium.org>
Date: Wed Jul 13 15:51:53 2016

Revert of arc: Use the new InstanceHolder for unittests (patchset #4 id:60001 of https://codereview.chromium.org/2138513002/ )

Reason for revert:
Caused unit_tests failure on Linux ChromiumOS Tests (1):

ArcAppModelBuilderTest.RemoveAppCleanUpFolder

Original issue's description:
> arc: Use the new InstanceHolder for unittests
>
> Instead of having unit tests go through Mojo and jump through hoops to
> ensure that all messages have propagated to all the threads, we can use
> the new arc::ArcBridgeService::InstanceHolder<T>::SetInstance() to
> directly set the concrete fake (and optional version). This simplifies
> testing quite a bit.
>
> BUG= 626695 
> TEST=trybots
>
> Committed: https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca
> Cr-Commit-Position: refs/heads/master@{#405138}

TBR=hidehiko@chromium.org,stevenjb@chromium.org,lhchavez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 626695 

Review-Url: https://codereview.chromium.org/2146573005
Cr-Commit-Position: refs/heads/master@{#405165}

[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service_impl.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service_impl.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/ui/arc/notification/arc_notification_manager_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 13 2016

Labels: merge-merged-2795
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/de2de961f2851a852febfb7223b0f27df5e6afb9

commit de2de961f2851a852febfb7223b0f27df5e6afb9
Author: lhchavez <lhchavez@chromium.org>
Date: Wed Jul 13 04:43:01 2016

arc: Revamp the ArcBridgeService interface

Adding a new host/instance pair required a lot of boilerplate. It also
made it very hard to properly do fakes for tests. This change adds two
more interfaces: InterfaceHolder<T> and InterfaceObserver<T>, so that
ArcBridgeService can stop having hundreds of methods to deal with
instance lifecycles.

BUG= 626695 
TEST=trybots

Review-Url: https://codereview.chromium.org/2133503002
Cr-Commit-Position: refs/heads/master@{#405025}

[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_downloads_watcher_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_navigation_throttle.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_policy_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_policy_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_policy_bridge_unittest.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_process_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_process_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_settings_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/arc_settings_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/gpu_arc_video_service_host.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/arc/gpu_arc_video_service_host.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/chromeos/file_manager/arc_file_tasks.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/memory/tab_manager_delegate_chromeos.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/memory/tab_manager_delegate_chromeos.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/task_management/providers/arc/arc_process_task.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/task_management/providers/arc/arc_process_task.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/app_list/arc/arc_app_utils.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc.gypi
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/BUILD.gn
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/arc_bridge_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/audio/arc_audio_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/audio/arc_audio_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/bluetooth/arc_bluetooth_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/bluetooth/arc_bluetooth_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/clipboard/arc_clipboard_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/clipboard/arc_clipboard_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/crash_collector/arc_crash_collector_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/crash_collector/arc_crash_collector_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/ime/arc_ime_bridge_impl.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/ime/arc_ime_bridge_impl.h
[add] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/instance_holder.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/activity_icon_loader.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/arc_intent_helper_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/arc_intent_helper_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/intent_helper/link_handler_model_impl.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/metrics/arc_metrics_service.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/metrics/arc_metrics_service.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/net/arc_net_host_impl.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/net/arc_net_host_impl.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/obb_mounter/arc_obb_mounter_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/obb_mounter/arc_obb_mounter_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/power/arc_power_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/power/arc_power_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/storage_manager/arc_storage_manager.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/storage_manager/arc_storage_manager.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/test/fake_policy_instance.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/test/fake_policy_instance.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/window_manager/arc_window_manager_bridge.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/components/arc/window_manager/arc_window_manager_bridge.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/BUILD.gn
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/notification/arc_notification_manager.h
[modify] https://crrev.com/de2de961f2851a852febfb7223b0f27df5e6afb9/ui/arc/notification/arc_notification_manager_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 13 2016

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

commit c5fffc64c14a0464f8870bbe91df7e945f42c3ca
Author: lhchavez <lhchavez@chromium.org>
Date: Wed Jul 13 14:01:41 2016

arc: Use the new InstanceHolder for unittests

Instead of having unit tests go through Mojo and jump through hoops to
ensure that all messages have propagated to all the threads, we can use
the new arc::ArcBridgeService::InstanceHolder<T>::SetInstance() to
directly set the concrete fake (and optional version). This simplifies
testing quite a bit.

BUG= 626695 
TEST=trybots

Review-Url: https://codereview.chromium.org/2138513002
Cr-Commit-Position: refs/heads/master@{#405138}

[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service_impl.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/arc_bridge_service_impl.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca/ui/arc/notification/arc_notification_manager_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 13 2016

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

commit 03b1e37434a736bbb5c764dd980787e54c93868b
Author: engedy <engedy@chromium.org>
Date: Wed Jul 13 15:51:53 2016

Revert of arc: Use the new InstanceHolder for unittests (patchset #4 id:60001 of https://codereview.chromium.org/2138513002/ )

Reason for revert:
Caused unit_tests failure on Linux ChromiumOS Tests (1):

ArcAppModelBuilderTest.RemoveAppCleanUpFolder

Original issue's description:
> arc: Use the new InstanceHolder for unittests
>
> Instead of having unit tests go through Mojo and jump through hoops to
> ensure that all messages have propagated to all the threads, we can use
> the new arc::ArcBridgeService::InstanceHolder<T>::SetInstance() to
> directly set the concrete fake (and optional version). This simplifies
> testing quite a bit.
>
> BUG= 626695 
> TEST=trybots
>
> Committed: https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca
> Cr-Commit-Position: refs/heads/master@{#405138}

TBR=hidehiko@chromium.org,stevenjb@chromium.org,lhchavez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 626695 

Review-Url: https://codereview.chromium.org/2146573005
Cr-Commit-Position: refs/heads/master@{#405165}

[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service_impl.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/arc_bridge_service_impl.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b/ui/arc/notification/arc_notification_manager_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 15 2016

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

commit 5aa0d0ed4da47326e168783b8251d8fbf0c58add
Author: lhchavez <lhchavez@chromium.org>
Date: Fri Jul 15 17:55:40 2016

Reland of arc: Use the new InstanceHolder for unittests (patchset #1 id:1 of https://codereview.chromium.org/2146573005/ )

Reason for revert:
https://codereview.chromium.org/2150583002 should fix the underlying issue that this change surfaced.

Original issue's description:
> Revert of arc: Use the new InstanceHolder for unittests (patchset #4 id:60001 of https://codereview.chromium.org/2138513002/ )
>
> Reason for revert:
> Caused unit_tests failure on Linux ChromiumOS Tests (1):
>
> ArcAppModelBuilderTest.RemoveAppCleanUpFolder
>
> Original issue's description:
> > arc: Use the new InstanceHolder for unittests
> >
> > Instead of having unit tests go through Mojo and jump through hoops to
> > ensure that all messages have propagated to all the threads, we can use
> > the new arc::ArcBridgeService::InstanceHolder<T>::SetInstance() to
> > directly set the concrete fake (and optional version). This simplifies
> > testing quite a bit.
> >
> > BUG= 626695 
> > TEST=trybots
> >
> > Committed: https://crrev.com/c5fffc64c14a0464f8870bbe91df7e945f42c3ca
> > Cr-Commit-Position: refs/heads/master@{#405138}
>
> TBR=hidehiko@chromium.org,stevenjb@chromium.org,lhchavez@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG= 626695 
>
> Committed: https://crrev.com/03b1e37434a736bbb5c764dd980787e54c93868b
> Cr-Commit-Position: refs/heads/master@{#405165}

Review-Url: https://codereview.chromium.org/2144013002
Cr-Commit-Position: refs/heads/master@{#405785}

[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/chrome/browser/ui/app_list/app_context_menu_unittest.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/chrome/browser/ui/app_list/arc/arc_app_test.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/chrome/browser/ui/app_list/arc/arc_app_unittest.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/arc_bridge_service.h
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/arc_bridge_service_impl.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/arc_bridge_service_impl.h
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/test/fake_app_instance.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/test/fake_app_instance.h
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/test/fake_notifications_instance.cc
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/components/arc/test/fake_notifications_instance.h
[modify] https://crrev.com/5aa0d0ed4da47326e168783b8251d8fbf0c58add/ui/arc/notification/arc_notification_manager_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-Request-53
These changes are needed for https://bugs.chromium.org/p/chromium/issues/detail?id=631253

Comment 10 by dimu@chromium.org, Jul 25 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 28 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63

commit f7ef8094f0b07aa9ff481d4af8087ec9a982bd63
Author: Luis Hector Chavez <lhchavez@google.com>
Date: Thu Jul 28 18:02:19 2016

arc: Revamp the ArcBridgeService interface

Adding a new host/instance pair required a lot of boilerplate. It also
made it very hard to properly do fakes for tests. This change adds two
more interfaces: InterfaceHolder<T> and InterfaceObserver<T>, so that
ArcBridgeService can stop having hundreds of methods to deal with
instance lifecycles.

BUG= 626695 
TEST=trybots

Review-Url: https://codereview.chromium.org/2133503002
Cr-Commit-Position: refs/heads/master@{#405025}
(cherry picked from commit de2de961f2851a852febfb7223b0f27df5e6afb9)

TBR=afakhry@chromium.org, georgesak@chromium.org, stevenjb@chromium.org, hidehiko@chromium.org

Review URL: https://codereview.chromium.org/2190483003 .

Cr-Commit-Position: refs/branch-heads/2785@{#389}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_auth_service.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_auth_service.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_downloads_watcher_service.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_downloads_watcher_service.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_navigation_throttle.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_policy_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_policy_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_policy_bridge_unittest.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_process_service.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_process_service.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_settings_service.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/arc_settings_service.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/gpu_arc_video_service_host.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/arc/gpu_arc_video_service_host.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/chromeos/file_manager/arc_file_tasks.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/memory/tab_manager_delegate_chromeos.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/memory/tab_manager_delegate_chromeos.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/task_management/providers/arc/arc_process_task.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/task_management/providers/arc/arc_process_task.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/ui/app_list/arc/arc_app_list_prefs.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/ui/app_list/arc/arc_app_utils.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc.gypi
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/BUILD.gn
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/arc_bridge_service.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/arc_bridge_service.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/audio/arc_audio_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/audio/arc_audio_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/bluetooth/arc_bluetooth_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/bluetooth/arc_bluetooth_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/clipboard/arc_clipboard_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/clipboard/arc_clipboard_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/crash_collector/arc_crash_collector_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/crash_collector/arc_crash_collector_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/ime/arc_ime_bridge_impl.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/ime/arc_ime_bridge_impl.h
[add] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/instance_holder.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/intent_helper/activity_icon_loader.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/intent_helper/arc_intent_helper_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/intent_helper/arc_intent_helper_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/intent_helper/link_handler_model_impl.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/metrics/arc_metrics_service.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/metrics/arc_metrics_service.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/net/arc_net_host_impl.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/net/arc_net_host_impl.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/obb_mounter/arc_obb_mounter_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/obb_mounter/arc_obb_mounter_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/power/arc_power_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/power/arc_power_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/storage_manager/arc_storage_manager.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/storage_manager/arc_storage_manager.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/test/fake_policy_instance.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/test/fake_policy_instance.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/window_manager/arc_window_manager_bridge.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/components/arc/window_manager/arc_window_manager_bridge.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/ui/arc/BUILD.gn
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/ui/arc/notification/arc_notification_manager.cc
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/ui/arc/notification/arc_notification_manager.h
[modify] https://crrev.com/f7ef8094f0b07aa9ff481d4af8087ec9a982bd63/ui/arc/notification/arc_notification_manager_unittest.cc

Sign in to add a comment