New issue
Advanced search Search tips

Issue 813234 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Cleanup ImageLoader helper process code

Project Member Reported by kerrnel@chromium.org, Feb 16 2018

Issue description

The helper process code in ImageLoader could use cleanup.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 20 2018

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

commit 09f06de246d2e767f5549baaca1a494e32560f1d
Author: Greg Kerr <kerrnel@chromium.org>
Date: Tue Feb 20 21:14:14 2018

imageloader: rename HelperProcess to HelperProcessProxy.

This renames the HelperProcess class to the more accurately named
HelperProcessProxy and cleans up some other style issues. It also
fixes an incorrect variable array usage. More cleanup CLs will follow.

BUG= chromium:813234 
TEST=auto_test platform_ImageLoaderServer

Change-Id: I8425b5a95dea5eee3ec1f71dda30857775e10aeb
Reviewed-on: https://chromium-review.googlesource.com/924653
Commit-Ready: Greg Kerr <kerrnel@chromium.org>
Tested-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>

[rename] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/helper_process_proxy.cc
[rename] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/helper_process_proxy.h
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader_impl.cc
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader_impl.h
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader_unittest.cc
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader_main.cc
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader.h
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/component.h
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader.gyp
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/component.cc
[rename] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/mock_helper_process_proxy.h
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/imageloader.cc
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/mount_helper.h
[modify] https://crrev.com/09f06de246d2e767f5549baaca1a494e32560f1d/imageloader/component_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 20 2018

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

commit 11eddb11f53287f664eaee1c6dbe2d0ae6aabb13
Author: Greg Kerr <kerrnel@chromium.org>
Date: Tue Feb 20 21:14:15 2018

imageloader: rename MountHelper to HelperProcessReceiver.

This renames the MountHelper class to the more clearly named
HelperProcessReceiver. It also addresses some C++ formatting issues.

BUG= chromium:813234 
TEST=auto_test platform_ImageLoaderServer

Change-Id: I19479ab9eb30da2d0aa9114c8eb8fd4972f223cf
Reviewed-on: https://chromium-review.googlesource.com/924683
Commit-Ready: Greg Kerr <kerrnel@chromium.org>
Tested-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/11eddb11f53287f664eaee1c6dbe2d0ae6aabb13/imageloader/imageloader_main.cc
[modify] https://crrev.com/11eddb11f53287f664eaee1c6dbe2d0ae6aabb13/imageloader/imageloader.gyp
[rename] https://crrev.com/11eddb11f53287f664eaee1c6dbe2d0ae6aabb13/imageloader/helper_process_receiver.cc
[rename] https://crrev.com/11eddb11f53287f664eaee1c6dbe2d0ae6aabb13/imageloader/helper_process_receiver.h

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 20 2018

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

commit 7a3a215f27d9f8ff6c3bea32e0c63db5005c61f3
Author: Greg Kerr <kerrnel@chromium.org>
Date: Tue Feb 20 21:14:16 2018

imageloader: Clean up protobuf serialization and deserialization.

This avoids extra data copying when the protobufs are serialized and
deserialized between the processes.

BUG= chromium:813234 
TEST=auto_test platform_ImageLoaderServer

Change-Id: I6c0e34b458f856ddcfabcd133d04d224030429e1
Reviewed-on: https://chromium-review.googlesource.com/924749
Commit-Ready: Greg Kerr <kerrnel@chromium.org>
Tested-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>

[modify] https://crrev.com/7a3a215f27d9f8ff6c3bea32e0c63db5005c61f3/imageloader/helper_process_proxy.cc
[modify] https://crrev.com/7a3a215f27d9f8ff6c3bea32e0c63db5005c61f3/imageloader/helper_process_receiver.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 1 2018

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

commit e2d0c191c19370a18f59f6a41aa3ddc021f05aec
Author: Greg Kerr <kerrnel@chromium.org>
Date: Thu Mar 01 20:39:11 2018

imageloader: Handle EINTR correctly.

This CL handles EINTR correctly when imageloader reads and writes to a
socket. It also causes the child process to abort on an unknown
operation.

BUG= chromium:813234 
TEST=auto_test platform_ImageLoaderServer

Change-Id: I63342d8901db5d760787e2502a29030cae09c8cc
Reviewed-on: https://chromium-review.googlesource.com/941910
Commit-Ready: Greg Kerr <kerrnel@chromium.org>
Tested-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/e2d0c191c19370a18f59f6a41aa3ddc021f05aec/imageloader/helper_process_proxy.cc
[modify] https://crrev.com/e2d0c191c19370a18f59f6a41aa3ddc021f05aec/imageloader/helper_process_receiver.cc

Status: Fixed (was: Started)

Sign in to add a comment