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

Issue 737148 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Task

Blocking:
issue 739118



Sign in to add a comment

Move socket fd for ArcBridgeService from Chrome to session_manager.

Project Member Reported by hidehiko@chromium.org, Jun 27 2017

Issue description

By moving the socket pair creation, we should be able to remove one async operation (= one state) in ArcSession, which can simplify the code a bit more.
 
Labels: -Type-Bug Type-Task
Cc: hidehiko@chromium.org
Owner: yusukes@chromium.org
Reassigned.
Labels: -Pri-3 M-61 Pri-2
Status: Started (was: Assigned)
Blocking: 739118
Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/137ec42a355f7800bb02739cb49a64e0c5e19f54

commit 137ec42a355f7800bb02739cb49a64e0c5e19f54
Author: Yusuke Sato <yusukes@google.com>
Date: Thu Jul 13 18:44:42 2017

Add |create_server_socket| to arc.proto

BUG= chromium:737148 
TEST=session_manager still compiles

Change-Id: Ie2dd300e0bd8f49372af79ec0f00c3c580b83d8c
Reviewed-on: https://chromium-review.googlesource.com/566339
Commit-Ready: Yusuke Sato <yusukes@chromium.org>
Tested-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/137ec42a355f7800bb02739cb49a64e0c5e19f54/dbus/login_manager/arc.proto

Project Member

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

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

commit 800f5e526a71690f37c751d722774f5d88bd3a4d
Author: yusukes <yusukes@chromium.org>
Date: Fri Jul 14 04:18:48 2017

Roll src/third_party/cros_system_api/ e79b0c77..137ec42a (2 commits).

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/e79b0c77..137ec42a

$ git log e79b0c77..137ec42a --date=short --no-merges --format='%ad %ae %s'
2017-07-10 yusukes@google.com Add |create_server_socket| to arc.proto
2017-06-13 ljusten@chromium.org authpolicy: Add common_name to ActiveDirectoryAccountInfo

BUG= chromium:737148 
TEST=try

Change-Id: Iba3d07cf332afa6b3596f508777a95ca9ae98378
Reviewed-on: https://chromium-review.googlesource.com/570884
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486672}
[modify] https://crrev.com/800f5e526a71690f37c751d722774f5d88bd3a4d/DEPS

Project Member

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

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/e69bdbb698f1c863e723fef99202af590005fd6c

commit e69bdbb698f1c863e723fef99202af590005fd6c
Author: Yusuke Sato <yusukes@google.com>
Date: Fri Jul 14 07:02:47 2017

login: Create arc_bridge.sock in session_manager

Previously, Chrome created the socket and then called the dbus
method for starting an ARC instance, but doing such a low level
file/socket operation in Chrome was a bit unusual. Chrome also
had to do the operation asynchronously with a blocking pool which
made the Chrome's state transition unnecessarily complicated.

With this CL, Chrome can simply ask session_manager to create a
socket and use it without any thread hops.

BUG= chromium:737148 
TEST=cros_run_unit_tests --board=caroline --packages
 chromeos-login
TEST=Also manually confirmed that ARC in all modes, login-screen,
 boot-continue, and from-scratch still work.
CQ-DEPEND=CL:566339

Change-Id: I77087a65ffb9ee7ee4c940aa31c54bb0cc69b003
Reviewed-on: https://chromium-review.googlesource.com/566139
Commit-Ready: Yusuke Sato <yusukes@chromium.org>
Tested-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/system_utils_impl.cc
[add] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/named_platform_handle_utils_posix.cc
[add] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/named_platform_handle_utils.h
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/session_manager_impl.h
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/session_manager_impl.cc
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/session_manager_impl_unittest.cc
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/login_manager.gyp
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/system_utils.h
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/system_utils_impl.h
[modify] https://crrev.com/e69bdbb698f1c863e723fef99202af590005fd6c/login_manager/mock_system_utils.h

Project Member

Comment 8 by bugdroid1@chromium.org, Jul 21 2017

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

commit 02df81bc8df1152947a779b6c190916a9a7cee56
Author: yusukes <yusukes@chromium.org>
Date: Fri Jul 21 00:27:02 2017

Immediately shut down the instance when creating a pipe fails

Previously, ArcSession reported GENERIC_BOOT_FAILURE to its observer,
but it didn't immediately shut down the container. This CL fixes the
issue.

BUG= chromium:737148 
TEST=ARC++ still starts

Change-Id: I8dce70a91c521909406594ac325e84765ee0cb7a
Reviewed-on: https://chromium-review.googlesource.com/579167
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488515}
[modify] https://crrev.com/02df81bc8df1152947a779b6c190916a9a7cee56/components/arc/arc_session.cc

I'm going to land https://chromium-review.googlesource.com/c/566247/ once M61 granch is created, then close this issue.
*branch

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 23 2017

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

commit f3dd555910d1e01f09f27c44cab8223fe8c27334
Author: Yusuke Sato <yusukes@chromium.org>
Date: Sun Jul 23 07:43:53 2017

Use StartArcInstance's |create_server_socket| feature

to simplify arc::ArcSessionImpl. This CL depends on
CL:566339 for session_manager and CL:566339 for system_api.

BUG= chromium:737148 
TEST=ARC in all modes, login-screen, boot-continue, and
 from-scratch still work.

Change-Id: I8d23840a330d789941a3deabbf55e5dba7460ecc
Reviewed-on: https://chromium-review.googlesource.com/566247
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488879}
[modify] https://crrev.com/f3dd555910d1e01f09f27c44cab8223fe8c27334/chromeos/dbus/fake_session_manager_client.cc
[modify] https://crrev.com/f3dd555910d1e01f09f27c44cab8223fe8c27334/chromeos/dbus/session_manager_client.cc
[modify] https://crrev.com/f3dd555910d1e01f09f27c44cab8223fe8c27334/chromeos/dbus/session_manager_client.h
[modify] https://crrev.com/f3dd555910d1e01f09f27c44cab8223fe8c27334/components/arc/arc_session.cc

Labels: -M-61 M-62
Status: Fixed (was: Started)
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/542963a5cdc83f23949f7701110d911712ee9953

commit 542963a5cdc83f23949f7701110d911712ee9953
Author: Yusuke Sato <yusukes@google.com>
Date: Thu Jul 27 05:41:57 2017

Mark |create_server_socket| as deprecated

for now so that the field won't be used any further.

BUG= chromium:737148 
TEST=None

Change-Id: Ibbb4adfe54897620dacf22696dc5b635a496b70b
Reviewed-on: https://chromium-review.googlesource.com/582707
Commit-Ready: Yusuke Sato <yusukes@chromium.org>
Tested-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/542963a5cdc83f23949f7701110d911712ee9953/dbus/login_manager/arc.proto

Project Member

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

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

commit e9abfa041e2d659ce2b36a5ef8454c5dc87e0217
Author: Yusuke Sato <yusukes@chromium.org>
Date: Fri Jul 28 04:43:16 2017

Roll src/third_party/cros_system_api/ dcb8a960..542963a5 (3 commits).

https://chromium.googlesource.com/chromiumos/platform/system_api.git/+log/dcb8a960..542963a5

$ git log dcb8a960..542963a5 --date=short --no-merges --format='%ad %ae %s'
2017-07-24 yusukes@google.com Mark |create_server_socket| as deprecated
2017-07-25 ejcaruso@chromium.org shill: add new MAC address randomization constants
2017-07-25 agable@chromium.org Remove system_api's unnecessary codereview.settings

BUG= chromium:737148 
TEST=try

Change-Id: If828ca45243af06824750d55ff15f2b4dbe9c29b
Reviewed-on: https://chromium-review.googlesource.com/588880
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490265}
[modify] https://crrev.com/e9abfa041e2d659ce2b36a5ef8454c5dc87e0217/DEPS

Project Member

Comment 15 by bugdroid1@chromium.org, Aug 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/d5e70979b3f1a7e9cf3a4270b9302c09bf1ee5e7

commit d5e70979b3f1a7e9cf3a4270b9302c09bf1ee5e7
Author: Yusuke Sato <yusukes@google.com>
Date: Thu Aug 10 06:33:06 2017

login: Create arc_bridge.sock in session_manager by default

to simplify SessionManagerImpl and its tests. This CL requires
Chrome >=r488879 (M62).

BUG= chromium:737148 
TEST=cros_run_unit_tests --board=caroline --packages
 chromeos-login

Change-Id: I753cf982a3b8cd5feea9dbf91b2bff0d37657a4e
Reviewed-on: https://chromium-review.googlesource.com/582061
Commit-Ready: Yusuke Sato <yusukes@chromium.org>
Tested-by: Yusuke Sato <yusukes@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>

[modify] https://crrev.com/d5e70979b3f1a7e9cf3a4270b9302c09bf1ee5e7/login_manager/session_manager_impl.cc
[modify] https://crrev.com/d5e70979b3f1a7e9cf3a4270b9302c09bf1ee5e7/login_manager/dbus_bindings/org.chromium.SessionManagerInterface.xml
[modify] https://crrev.com/d5e70979b3f1a7e9cf3a4270b9302c09bf1ee5e7/login_manager/session_manager_impl_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 24 2017

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

commit 825216a4cc4b36e4ab38013e15b4134a4243f91d
Author: yusukes <yusukes@chromium.org>
Date: Thu Aug 24 20:02:39 2017

Stop setting |create_server_socket| in SessionManagerClientImpl

Since CL:582061 (Chrome OS 9827.0.0) made it the default behavior,
Chrome is no longer required to set the flag.

BUG= chromium:737148 
TEST=ARC still starts

Change-Id: I5ebc237869834eb4f792c12fefdcce028815a24c
Reviewed-on: https://chromium-review.googlesource.com/633906
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497168}
[modify] https://crrev.com/825216a4cc4b36e4ab38013e15b4134a4243f91d/chromeos/dbus/session_manager_client.cc

Sign in to add a comment