[components] Make zip a component so that src/chromeos can use it.

[components] Make zip a component so that src/chromeos can use it.

BUG=180711
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192843

[Jói, 2013-04-03 15:42:56]

(digging through one week's worth of email)

I think I saw a discussion elsewhere that maybe this should move to
base/ somewhere, and without looking in any detail at the code in
question, that sort of makes sense to me.  Based on this, for now I
won't review.  Let me know if this is in error.

Cheers,
Jói


On Thu, Mar 28, 2013 at 10:28 PM,  <phajdan.jr@chromium.org> wrote:
> Reviewers: Jói,
>
> Description:
> [components] Make zip a component so that src/chromeos can use it.
>
> BUG=180711
>
>
> Please review this at https://codereview.chromium.org/13257004/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>   M chrome/DEPS
>   M chrome/browser/chromeos/imageburner/burn_manager.cc
>   M chrome/browser/component_updater/component_unpacker.cc
>   M chrome/browser/extensions/extension_creator.cc
>   M chrome/browser/feedback/feedback_data.cc
>   M chrome/browser/safe_browsing/download_protection_service_unittest.cc
>   M chrome/chrome_common.gypi
>   M chrome/chrome_tests_unit.gypi
>   M chrome/common/extensions/unpacker.cc
>   M chrome/common/safe_browsing/zip_analyzer.cc
>   D chrome/common/zip.h
>   D chrome/common/zip.cc
>   D chrome/common/zip_internal.h
>   D chrome/common/zip_internal.cc
>   D chrome/common/zip_reader.h
>   D chrome/common/zip_reader.cc
>   D chrome/common/zip_reader_unittest.cc
>   D chrome/common/zip_unittest.cc
>   D chrome/test/data/zip/create_test_zip.sh
>   D chrome/test/data/zip/evil.zip
>   D chrome/test/data/zip/evil_via_absolute_file_name.zip
>   D chrome/test/data/zip/evil_via_invalid_utf8.zip
>   D chrome/test/data/zip/test.zip
>   D chrome/test/data/zip/test/foo.txt
>   D chrome/test/data/zip/test/foo/bar.txt
>   D chrome/test/data/zip/test/foo/bar/.hidden
>   D chrome/test/data/zip/test/foo/bar/baz.txt
>   D chrome/test/data/zip/test/foo/bar/quux.txt
>   D chrome/test/data/zip/test_nocompress.zip
>   M chrome/test/webdriver/webdriver_capabilities_parser_unittest.cc
>   M chrome/test/webdriver/webdriver_util.cc
>   M chrome/utility/chrome_content_utility_client.cc
>   M components/components.gyp
>   M components/components_tests.gypi
>   A + components/test/data/zip/create_test_zip.sh
>   A + components/test/data/zip/evil.zip
>   A + components/test/data/zip/evil_via_absolute_file_name.zip
>   A + components/test/data/zip/evil_via_invalid_utf8.zip
>   A + components/test/data/zip/test.zip
>   A + components/test/data/zip/test/foo.txt
>   A + components/test/data/zip/test/foo/bar.txt
>   A + components/test/data/zip/test/foo/bar/.hidden
>   A + components/test/data/zip/test/foo/bar/baz.txt
>   A + components/test/data/zip/test/foo/bar/quux.txt
>   A + components/test/data/zip/test_nocompress.zip
>   A + components/zip.gypi
>   A components/zip/DEPS
>   A + components/zip/zip.h
>   A + components/zip/zip.cc
>   A + components/zip/zip_internal.h
>   A + components/zip/zip_internal.cc
>   A + components/zip/zip_reader.h
>   A + components/zip/zip_reader.cc
>   A + components/zip/zip_reader_unittest.cc
>   A + components/zip/zip_unittest.cc
>
>

[Paweł Hajdan Jr., 2013-04-03 16:03:04]

On 2013/04/03 15:42:56, Jói wrote:
> (digging through one week's worth of email)
> 
> I think I saw a discussion elsewhere that maybe this should move to
> base/ somewhere, and without looking in any detail at the code in
> question, that sort of makes sense to me.  Based on this, for now I
> won't review.  Let me know if this is in error.

Brett Wilson does not want it in base. src/components has been suggested as an
alternative (in fact, I think I've also listed it as one of possible places).

Note that this is the first thing I'm moving to components, so feedback is
welcome about how specifically things should be done.

[Jói, 2013-04-04 11:58:57]

LGTM with a nit.

https://codereview.chromium.org/13257004/diff/1/components/zip/DEPS
File components/zip/DEPS (right):

https://codereview.chromium.org/13257004/diff/1/components/zip/DEPS#newcode1
components/zip/DEPS:1: include_rules = [
can you add "-content" to avoid any dependency on that in this component (the
top-level //components/DEPS file allows content/public/common since that is
generally OK for stuff under //components).

[Jói, 2013-04-04 12:00:30]

One more thing:  It would be good to figure out one or two people who
could be added to OWNERS for //components/zip.  Probably just look at
git blame for the implementation files and check with the candidates
you find.

Cheers,
Jói


On Thu, Apr 4, 2013 at 11:58 AM,  <joi@chromium.org> wrote:
> LGTM with a nit.
>
>
> https://codereview.chromium.org/13257004/diff/1/components/zip/DEPS
> File components/zip/DEPS (right):
>
> https://codereview.chromium.org/13257004/diff/1/components/zip/DEPS#newcode1
> components/zip/DEPS:1: include_rules = [
> can you add "-content" to avoid any dependency on that in this component
> (the top-level //components/DEPS file allows content/public/common since
> that is generally OK for stuff under //components).
>
> https://codereview.chromium.org/13257004/

[Paweł Hajdan Jr., 2013-04-04 23:33:33]

DEPS and OWNERS updated.

Satoru, Haixia: OK to add you as src/components/zip/OWNERS?

If not, please suggest suitable OWNERS.

[satorux1, 2013-04-05 01:30:08]

Thank you for moving files. I'm file with in the OWNERS file. IIRC, the original
code was written by aa@. Aaron, do you want to be in the OWNERS file?

[hshi1, 2013-04-05 22:20:32]

On 2013/04/05 01:30:08, satorux1 wrote:
> Thank you for moving files. I'm file with in the OWNERS file. IIRC, the
original
> code was written by aa@. Aaron, do you want to be in the OWNERS file?

I'm fine with being in the OWNERS file. Thanks!

[Paweł Hajdan Jr., 2013-04-08 18:30:02]

Committed patchset #2 manually as r192843.