New issue
Advanced search Search tips

Issue 889703 link

Starred by 2 users
Status: Fixed
Owner:
amistry@chromium.org
Closed: Nov 2
Cc:
noel@chromium.org
slangley@chromium.org
yamaguchi@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 892478
Blocking:
issue 891116



Sign in to add a comment

Clean up zip archiver code.

Project Member Reported by amistry@chromium.org, Sep 27 
Chrome Version: M71

The zip archiver code is part of the chromium code base and built using GN (using //base), and should therefore be updated to follow chromium style. This includes:
- Following Chromium c++ style
- //base libraries for primitives such as threading
- using std::unique_ptr where possible and proper object ownership and lifecycles
- IWYU
- Unit tests
- etc...
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 27 

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

commit 2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449
Author: Anand K. Mistry <amistry@chromium.org>
Date: Thu Sep 27 07:27:21 2018

Move constants out of headers and into implementation files.

As a result, also use std::unique_ptr<char[]> instead of char[] class
members. This makes zip archiver more closely follow chromium style.

BUG= 889703 

Change-Id: I551557958beff280e344134f15be20a406fb5ea8
Reviewed-on: https://chromium-review.googlesource.com/1242643
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594621}
[modify] https://crrev.com/2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip.cc
[modify] https://crrev.com/2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip.h
[modify] https://crrev.com/2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_io_javascript_stream.cc
[modify] https://crrev.com/2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_io_javascript_stream.h
[modify] https://crrev.com/2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc
[modify] https://crrev.com/2a6ea0f9e0e91dd282cd5cd2c612e7fa2c5e0449/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.h
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 27 

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

commit d970c082c0fe7a33af1afa7af1d1058a3d0b2342
Author: Anand K. Mistry <amistry@chromium.org>
Date: Thu Sep 27 23:46:48 2018

Lazily load the zip archiver's NaCl module.

The zip archiver component extension already handles lazy loading of the
NaCl module, but it was also unnecessarily loading it on startup.

BUG= 889703 

Change-Id: I7346a13d4a317cd62e6b7a0f3e41870b723be28e
Reviewed-on: https://chromium-review.googlesource.com/1248343
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594919}
[modify] https://crrev.com/d970c082c0fe7a33af1afa7af1d1058a3d0b2342/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/d970c082c0fe7a33af1afa7af1d1058a3d0b2342/chrome/browser/resources/chromeos/zip_archiver/js/background.js
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 28 

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

commit c16c1fc9fb925add66e82a98b522cf667f1f607d
Author: Anand K. Mistry <amistry@chromium.org>
Date: Fri Sep 28 00:00:02 2018

Use std::unique_ptr instead of raw pointers.

BUG= 889703 

Change-Id: I3280a573a6d13c9dc386758b1c177e07bec84e7c
Reviewed-on: https://chromium-review.googlesource.com/1247603
Commit-Queue: Anand Mistry <amistry@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594922}
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor.cc
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor.h
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_io_javascript_stream.cc
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_io_javascript_stream.h
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/module.cc
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/volume.cc
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/volume.h
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive.h
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc
[modify] https://crrev.com/c16c1fc9fb925add66e82a98b522cf667f1f607d/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.h
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 28 

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

commit 62b48b4fdb3984762f4d2fd3690f02f167920f52
Author: Anand K. Mistry <amistry@chromium.org>
Date: Fri Sep 28 03:36:36 2018

Clean up to allow zip archiver to compile using the non-NaCl toolchain.

These changes are necessary to eventually integrate the zip archivers
unit tests.

Changes include:
- virtual -> override
- Un-inlining overridden functions
- Correcting types

BUG= 889703 

Change-Id: I83030c6b77dda026c4a3fee50df2ecbcedb898f6
Reviewed-on: https://chromium-review.googlesource.com/1250281
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594972}
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/BUILD.gn
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor.cc
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip.h
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_io_javascript_stream.h
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume.cc
[add] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive.cc
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive.h
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.h
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/unpacker-test/cpp/fake_volume_reader.cc
[modify] https://crrev.com/62b48b4fdb3984762f4d2fd3690f02f167920f52/chrome/browser/resources/chromeos/zip_archiver/unpacker-test/cpp/fake_volume_reader.h
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 28 

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

commit aed664953a642da8ba0f5ed4b6f05f843168ce1f
Author: Noel Gordon <noel@chromium.org>
Date: Fri Sep 28 04:08:28 2018

Remove ZipTest browser test special case

The zip archiver NaCl module is lazily loaded crrev.com/594919. Remove
the browser_test special case handling added to control when that NaCl
loaded (only for zip tests); lazy loading should do that for us.

Bug:  889703 
Change-Id: I726f32849a6278765a4932a405328faff78bd594
Reviewed-on: https://chromium-review.googlesource.com/1250341
Reviewed-by: Anand Mistry <amistry@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594977}
[modify] https://crrev.com/aed664953a642da8ba0f5ed4b6f05f843168ce1f/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/aed664953a642da8ba0f5ed4b6f05f843168ce1f/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/aed664953a642da8ba0f5ed4b6f05f843168ce1f/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Comment 8 by noel@chromium.org, Oct 2

Cc: noel@chromium.org
 Issue 891302  has been merged into this issue.

Comment 9 by noel@chromium.org, Oct 2 

 Issue 891116  has been merged into this issue.

Comment 10 by noel@chromium.org, Oct 2 

FindIt complained: all ZipFile tests got disabled in  issue 891116 .

Comment 11 by noel@chromium.org, Oct 2

Labels: CrOSFilesCategory-Testing

Comment 13 by noel@chromium.org, Oct 2 

Two issues:

 1) TIMEOUT PASS flakes
    need to make the NaCl compilation step happen early in test so we don't
    flirt with the bot test time-out.

 2) CRASH PASS flakes
[1:1:1001/155952.046875:FATAL:find_paint_offset_and_visual_rect_needing_update.h(72)] Check failed: context.tree_builder_context_actually_needed_.
    seems to be an unrelated issues, but it is  issue 890969

Comment 15 by noel@chromium.org, Oct 3 

k, maybe report ^^^ that onto  issue 890969  ?
Project Member

Comment 16 by Findit, Oct 3

Flaky-Test: ZipFiles/FilesAppBrowserTest.Test
Labels: Type-Bug Test-Flaky Test-Findit-Detected Sheriff-Chromium

ZipFiles/FilesAppBrowserTest.Test?* is flaky.

Findit has detected 63 new flake occurrences of this test. List
of all flake occurrences can be found at:
https://findit-for-me.appspot.com/flake/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSBUZsYWtlIjhjaHJvbWl1bUBicm93c2VyX3Rlc3RzQFppcEZpbGVzL0ZpbGVzQXBwQnJvd3NlclRlc3QuVGVzdAw.

Since this test is still flaky, this issue has been moved back onto the Sheriff
Bug Queue if it's not already there.

This flaky test was previously tracked in  bug 891116 .

If the result above is wrong, please file a bug using this link:
https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Detection%20-%20Wrong%20result%20for%20ZipFiles/FilesAppBrowserTest.Test&comment=Link%20to%20flake%20occurrences%3A%20https://findit-for-me.appspot.com/flake/occurrences?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSBUZsYWtlIjhjaHJvbWl1bUBicm93c2VyX3Rlc3RzQFppcEZpbGVzL0ZpbGVzQXBwQnJvd3NlclRlc3QuVGVzdAw

Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).

Comment 17 by noel@chromium.org, Oct 3 

#14 > maybe report ^^^ that onto  issue 890969  ?

Done. And Findit found the Providers test failures ( issue 891119 ).
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 4 

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

commit f2d4c81bf6da021667e630e04e9b5cf8f5f9acf8
Author: Anand K. Mistry <amistry@chromium.org>
Date: Thu Oct 04 01:31:25 2018

Preload zip archiver NaCl module for zip browser tests.

In DEBUG builds, the PNaCl translation takes a significant amount of
time, causing zip browser test TIMEOUT flakes on the bots.

Recover the FilesApp browser test feature used to declare ZipTests and
for the zip browser tests, tell the zip-archiver extension to preload
the NaCl module before it's needed, so its translation starts as early
as possible during zip browser tests.

BUG= 889703 

Change-Id: Ic16a4b4e53a4be88bae8ee5d2c2caa8baaa1487b
Reviewed-on: https://chromium-review.googlesource.com/c/1257464
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596470}
[modify] https://crrev.com/f2d4c81bf6da021667e630e04e9b5cf8f5f9acf8/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/f2d4c81bf6da021667e630e04e9b5cf8f5f9acf8/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/f2d4c81bf6da021667e630e04e9b5cf8f5f9acf8/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h
[modify] https://crrev.com/f2d4c81bf6da021667e630e04e9b5cf8f5f9acf8/chrome/browser/resources/chromeos/zip_archiver/js/background.js

Comment 19 by sky@chromium.org, Oct 4

Labels: -Pri-3 Pri-1
Bumping to Pri-1 since this is blocking Pri-1 bugs. This is the top cause of test flakiness right now according to Ranked Flaky Tests: https://findit-for-me.appspot.com/ranked-flakes

Comment 20 by amistry@chromium.org, Oct 5 

sky: Zip tests have been disabled, and the failures are caused by a blink issue (see stack trace in 890969, comment #2) which affects non-zip tests too.

Comment 21 by amistry@chromium.org, Oct 5

Blockedon: 892478
Filed  issue 892478  for the blink crash.

Comment 22 by sky@chromium.org, Oct 5 

amistry, thanks for updating this bug with details. Sorry for the noise.

Comment 23 by sky@chromium.org, Oct 5

Labels: -Pri-1 Pri-3
I'm lowering back to a 3, amistry, feel free to adjust as necessary.

Comment 24 by noel@chromium.org, Oct 8

Blocking: 891116
Project Member

Comment 25 by bugdroid1@chromium.org, Oct 9 

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

commit cbe1de8f928dea212b1963faae94e020b4cc58e7
Author: Anand K. Mistry <amistry@chromium.org>
Date: Tue Oct 09 22:08:02 2018

Re-enable ZipFiles and Providers file manager browser tests

These tests were previously timing out due to a renderer crash. That bug
( https://crbug.com/892478 ) has been fixed, so re-enable these tests.

BUG= 889703 , 891119 

Change-Id: Ie399bbc89e3ffaceb38c79e180a08b44faa080ce
Reviewed-on: https://chromium-review.googlesource.com/c/1271735
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598102}
[modify] https://crrev.com/cbe1de8f928dea212b1963faae94e020b4cc58e7/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Comment 26 by vasi...@chromium.org, Oct 11

Labels: -Sheriff-Chromium
Project Member

Comment 29 by Findit, Oct 12

Flaky-Test: FilesAppBrowserTest.Test
Labels: Test-Findit-Analyzed

Flaky test: FileDisplay/FilesAppBrowserTest.Test/fileDisplayWithoutVolumesThenMountDrive_DriveFs
Test output log: https://chromium-swarm.appspot.com/task?id=407fb2a571b3c510
Findit identified the culprit r599026 with confidence 100.0% for the example failed
build https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/9016 based on the flakiness trend:

https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy6gELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKzAWNocm9taXVtLm1lbW9yeS9MaW51eCBDaHJvbWl1bU9TIE1TYW4gVGVzdHMvOTAxNi92aXpfYnJvd3Nlcl90ZXN0cy9SbWxzWlVScGMzQnNZWGt2Um1sc1pYTkJjSEJDY205M2MyVnlWR1Z6ZEM1VVpYTjBMMlpwYkdWRWFYTndiR0Y1VjJsMGFHOTFkRlp2YkhWdFpYTlVhR1Z1VFc5MWJuUkVjbWwyWlY5RWNtbDJaVVp6DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw

If the culprit above is wrong, please file a bug using this link:
https://bugs.chromium.org/p/chromium/issues/entry?status=Unconfirmed&labels=Pri-1,Test-Findit-Wrong&components=Tools%3ETest%3EFindit%3EFlakiness&summary=%5BFindit%5D%20Flake%20Analyzer%20-%20Wrong%20result%20for%20FileDisplay/FilesAppBrowserTest.Test/fileDisplayWithoutVolumesThenMountDrive_DriveFs&comment=Link%20to%20Analysis%3A%20https://findit-for-me.appspot.com/waterfall/flake?key=ag9zfmZpbmRpdC1mb3ItbWVy6gELEhdNYXN0ZXJGbGFrZUFuYWx5c2lzUm9vdCKzAWNocm9taXVtLm1lbW9yeS9MaW51eCBDaHJvbWl1bU9TIE1TYW4gVGVzdHMvOTAxNi92aXpfYnJvd3Nlcl90ZXN0cy9SbWxzWlVScGMzQnNZWGt2Um1sc1pYTkJjSEJDY205M2MyVnlWR1Z6ZEM1VVpYTjBMMlpwYkdWRWFYTndiR0Y1VjJsMGFHOTFkRlp2YkhWdFpYTlVhR1Z1VFc5MWJuUkVjbWwyWlY5RWNtbDJaVVp6DAsSE01hc3RlckZsYWtlQW5hbHlzaXMYAQw

Automatically posted by the findit-for-me app (https://goo.gl/Ot9f7N).
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 12 

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

commit 402571f89e1aff1627ccf491110764c74029e3de
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Oct 12 13:17:58 2018

Revert "Files app: Select My files when there are no volumes"

This reverts commit 5cd8c123e452c0b76fbbcaf53e82d86ca7d54930.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 599026 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vNWNkOGMxMjNlNDUyYzBiNzZmYmJjYWY1M2U4MmQ4NmNhN2Q1NDkzMAw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Linux%20ChromiumOS%20MSan%20Tests/9016

Sample Failed Step: viz_browser_tests

Sample Flaky Test: FileDisplay/FilesAppBrowserTest.Test/fileDisplayWithoutVolumesThenMountDrive_DriveFs

Original change's description:
> Files app: Select My files when there are no volumes
> 
> Make Files app select "My files" when there are no available volumes,
> this to allow Files app to behave properly when volumes subsequently
> become available.
> 
> Change DirectoryModel.onVolumeInfoListUpdated_ method to check for
> non-null |displayRoot| before trying to change to |displayRoot|. This
> fixes the error "Cannot read property 'getParent' of null" when Drive
> volume becomes available before Downloads volume, which is the default
> volume/root.
> 
> Change FakeDriveFs to unbind the two mojo bindings if they're bound, so
> DriveFsTestVolume can re-mount itself.
> 
> Test: browser_tests --gtest_filter="*/fileDisplayWithoutVolumesThenMount*"
> Bug: 893161,  884967 
> Change-Id: Ic813b25261530495c11c9f641a92f6e07f883702
> Reviewed-on: https://chromium-review.googlesource.com/c/1272418
> Reviewed-by: Noel Gordon <noel@chromium.org>
> Reviewed-by: Sam McNally <sammc@chromium.org>
> Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599026}

Change-Id: I883485c8fc1bdcc22dba93cc4b03b7c157dcb5f2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 893161,  884967 ,  889703 
Reviewed-on: https://chromium-review.googlesource.com/c/1278063
Cr-Commit-Position: refs/heads/master@{#599190}
[modify] https://crrev.com/402571f89e1aff1627ccf491110764c74029e3de/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/402571f89e1aff1627ccf491110764c74029e3de/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/402571f89e1aff1627ccf491110764c74029e3de/chromeos/components/drivefs/fake_drivefs.cc
[modify] https://crrev.com/402571f89e1aff1627ccf491110764c74029e3de/ui/file_manager/file_manager/foreground/js/directory_model.js
[modify] https://crrev.com/402571f89e1aff1627ccf491110764c74029e3de/ui/file_manager/file_manager/foreground/js/file_manager.js
[modify] https://crrev.com/402571f89e1aff1627ccf491110764c74029e3de/ui/file_manager/integration_tests/file_manager/file_display.js
Project Member

Comment 31 by bugdroid1@chromium.org, Oct 12 

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

commit c8e7788f4f786a59d39bbe9720548f5ac796d507
Author: Anand K. Mistry <amistry@chromium.org>
Date: Fri Oct 12 23:27:43 2018

Create a unit test for the zip archiver.

This change only has basic tests for zip unpacking, and none for zip
packing. The intention is to be a starting point for creating more
tests.

The test binary won't be run on any bots. Eventually, the unit tests
should be folded into the unit_tests target, but that is not currently
possible due to symbol collisions caused by using two different, and
incompatible, versions of minizip.

BUG= 889703 
TBR=mtomasz@chromium.org

Change-Id: Ie9ae2b3650a552c02097d16652304853bfd4a4f8
Reviewed-on: https://chromium-review.googlesource.com/c/1272417
Commit-Queue: Anand Mistry <amistry@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Scott Graham <scottmg@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599411}
[modify] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/BUILD.gn
[modify] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/chrome/browser/resources/chromeos/zip_archiver/cpp/BUILD.gn
[modify] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip.cc
[add] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/chrome/browser/resources/chromeos/zip_archiver/cpp/test_module.cc
[modify] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc
[add] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip_unittest.cc
[rename] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/chrome/browser/resources/chromeos/zip_archiver/test/data/small_zip.zip
[modify] https://crrev.com/c8e7788f4f786a59d39bbe9720548f5ac796d507/third_party/minizip/BUILD.gn
Project Member

Comment 32 by bugdroid1@chromium.org, Oct 15 

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

commit fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45
Author: Anand K. Mistry <amistry@chromium.org>
Date: Mon Oct 15 20:30:39 2018

Use base::Time for modification time instead of a plain int64_t

Also, update the comment about the 'modification_time' field in the
compression request.

BUG= 889703 

Change-Id: I37df9e8abc50b195b73ffd9cf101444dc682ff33
Reviewed-on: https://chromium-review.googlesource.com/c/1279199
Commit-Queue: Anand Mistry <amistry@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599735}
[modify] https://crrev.com/fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor.cc
[modify] https://crrev.com/fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive.h
[modify] https://crrev.com/fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip.cc
[modify] https://crrev.com/fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip.h
[modify] https://crrev.com/fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45/chrome/browser/resources/chromeos/zip_archiver/cpp/request.h
[modify] https://crrev.com/fd2b89cdfe7ac055eaea12776fdf7b7c5496fb45/chrome/browser/resources/chromeos/zip_archiver/js/request.js

Comment 33 by amistry@chromium.org, Oct 16

Flaky-Test: ----
Labels: -Test-Flaky -Test-Findit-Analyzed -Test-Findit-Detected
-bunch of labels so findit doesn't inappropriately add stuff to this bug.
Project Member

Comment 34 by bugdroid1@chromium.org, Oct 16 

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

commit b68705b2717bf99281dee5908badf70f188e7600
Author: Anand K. Mistry <amistry@chromium.org>
Date: Tue Oct 16 19:08:58 2018

Add unittests for zip creation.

BUG= 415871 , 889703 

Change-Id: Ifa76627a000b66f1e77aa5d2229a81bf470e7d55
Reviewed-on: https://chromium-review.googlesource.com/c/1282038
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600066}
[modify] https://crrev.com/b68705b2717bf99281dee5908badf70f188e7600/chrome/browser/resources/chromeos/zip_archiver/cpp/BUILD.gn
[add] https://crrev.com/b68705b2717bf99281dee5908badf70f188e7600/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip_unittest.cc
Project Member

Comment 36 by bugdroid1@chromium.org, Oct 30 

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

commit 323ea323ac45e8a24d86aa710b5769f29d8f4fca
Author: Anand K. Mistry <amistry@chromium.org>
Date: Tue Oct 30 05:44:58 2018

Use base::Optional for passphrase, and add a unit test.

BUG= 889703 

Change-Id: I06284ea5d72359ea9d6950b65aa891e86b20550e
Reviewed-on: https://chromium-review.googlesource.com/c/1307016
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Anand Mistry <amistry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603799}
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/compressor_archive_minizip_unittest.cc
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.cc
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip.h
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_archive_minizip_unittest.cc
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader.h
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.cc
[modify] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/cpp/volume_reader_javascript_stream.h
[rename] https://crrev.com/323ea323ac45e8a24d86aa710b5769f29d8f4fca/chrome/browser/resources/chromeos/zip_archiver/test/data/encrypted.zip

Comment 37 by amistry@chromium.org, Nov 2

Status: Fixed (was: Assigned)
►

Sign in to add a comment