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

Issue 773310 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ZipFileCreator should use the service manager

Project Member Reported by jcivelli@chromium.org, Oct 10 2017

Issue description

As part of the effort to move process launch to the service manager, ZipFileCreator should use the service manager instead of utility_process_mojo_client.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 18 2017

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

commit d0cb5e408404d652492171bbed9c8ecd3d44a9aa
Author: Jay Civelli <jcivelli@google.com>
Date: Wed Oct 18 05:30:37 2017

Changing the FileAccessor API in zip.h to improve perfs over IPC.

When using zip::Zip() with an IPC based FileAccessor, zipping
directories with large number of files triggers many IPC calls
making the entire operation significantly slower than with direct file
access.
In order to alleviate this performance hit, this patch groups file
reads by modifying the FileAccessor read method so it reads multiple
files at once. zip::Zip() can then group these reads when writing the
ZIP file.
The writing code has been factored out into a new ZipWriter class to
make that code more readable.

Bug:  773310 
Change-Id: I8121980bf05d87a174c63164840ec6bf325c7e52
Reviewed-on: https://chromium-review.googlesource.com/719356
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509693}
[modify] https://crrev.com/d0cb5e408404d652492171bbed9c8ecd3d44a9aa/third_party/zlib/google/BUILD.gn
[modify] https://crrev.com/d0cb5e408404d652492171bbed9c8ecd3d44a9aa/third_party/zlib/google/zip.cc
[modify] https://crrev.com/d0cb5e408404d652492171bbed9c8ecd3d44a9aa/third_party/zlib/google/zip.h
[modify] https://crrev.com/d0cb5e408404d652492171bbed9c8ecd3d44a9aa/third_party/zlib/google/zip_unittest.cc
[add] https://crrev.com/d0cb5e408404d652492171bbed9c8ecd3d44a9aa/third_party/zlib/google/zip_writer.cc
[add] https://crrev.com/d0cb5e408404d652492171bbed9c8ecd3d44a9aa/third_party/zlib/google/zip_writer.h

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 2 2017

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

commit 8743364b8c74e19a062225e5e5de1d7fd174de86
Author: Jay Civelli <jcivelli@google.com>
Date: Thu Nov 02 18:14:36 2017

Servicifying ZipFileCreator.

Servicifying ZipFileCreator so it uses the service manager directly
instead of using utility_process_mojo_client.
Made it a chrome service in a new chrome/services directory that will
host more chrome specific services in the future.
It's part of the chrome_file_util service which will include other
file related operations.

select "Zip selection". A valid archive should be created.

Bug:  773310 
Test: in the File app, select one or more files then right click and
Change-Id: I1a354c7044734b12d471adedf888b447be8311cc
Reviewed-on: https://chromium-review.googlesource.com/708517
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Daichi Hirono <hirono@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513554}
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/app/BUILD.gn
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/app/generated_resources.grd
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/browser/DEPS
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/browser/chrome_content_utility_manifest_overlay.json
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/browser/chromeos/extensions/file_manager/private_api_misc.cc
[delete] https://crrev.com/4d3dff9b366d0535b8c08eeb71636dd198ad30bd/chrome/browser/chromeos/file_manager/zip_file_creator.cc
[delete] https://crrev.com/4d3dff9b366d0535b8c08eeb71636dd198ad30bd/chrome/browser/chromeos/file_manager/zip_file_creator_browsertest.cc
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/common/BUILD.gn
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/DEPS
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/BUILD.gn
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/DEPS
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/OWNERS
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/file_util_service.cc
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/file_util_service.h
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/manifest.json
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/cpp/BUILD.gn
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/cpp/DEPS
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/cpp/zip_file_creator.cc
[rename] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/cpp/zip_file_creator.h
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/cpp/zip_file_creator_browsertest.cc
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/interfaces/BUILD.gn
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/interfaces/OWNERS
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/interfaces/constants.mojom
[rename] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/public/interfaces/zip_file_creator.mojom
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/zip_file_creator.cc
[add] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/services/file_util/zip_file_creator.h
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/test/BUILD.gn
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/utility/BUILD.gn
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/utility/DEPS
[modify] https://crrev.com/8743364b8c74e19a062225e5e5de1d7fd174de86/chrome/utility/chrome_content_utility_client.cc

Status: Fixed (was: Untriaged)

Sign in to add a comment