New issue
Advanced search Search tips

Issue 833834 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Minor FileManager code clean and comment fixes

Project Member Reported by noel@chromium.org, Apr 17 2018

Issue description

Various clean-up opportunities were seen in the browser test code and in the JS files related to patch [1] used for fixing  issue 831074 .

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1013823

Shave off anything useful from the patches in [1] as separate code review and review and land.




 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 18 2018

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

commit 2c9c29527ada10af4745ab26dd000ebb6d5e055e
Author: Noel Gordon <noel@chromium.org>
Date: Wed Apr 18 04:42:32 2018

Add comments and Closure annotations from the JS fix for  issue 831074 

 Issue 831074  was resolved with a C++ fix, but there was a JS fix patch
prior to that that also fixed the problem. It was abandoned [1] but it
contained useful new comments + Closure annotations. Let's use them.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1013823/18

Bug:  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ic33a6f90ba99eedebc631dae09cb8bc45040fac9
Reviewed-on: https://chromium-review.googlesource.com/1014822
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551581}
[modify] https://crrev.com/2c9c29527ada10af4745ab26dd000ebb6d5e055e/ui/file_manager/audio_player/js/background.js
[modify] https://crrev.com/2c9c29527ada10af4745ab26dd000ebb6d5e055e/ui/file_manager/file_manager/background/js/background.js
[modify] https://crrev.com/2c9c29527ada10af4745ab26dd000ebb6d5e055e/ui/file_manager/file_manager/background/js/test_util_base.js
[modify] https://crrev.com/2c9c29527ada10af4745ab26dd000ebb6d5e055e/ui/file_manager/gallery/js/background.js
[modify] https://crrev.com/2c9c29527ada10af4745ab26dd000ebb6d5e055e/ui/file_manager/video_player/js/background.js

Project Member

Comment 2 by bugdroid1@chromium.org, Apr 18 2018

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

commit 7b2aadcb28ffc764aecba1ebfc1b1916e9da5ff8
Author: Noel Gordon <noel@chromium.org>
Date: Wed Apr 18 12:52:24 2018

FileManagerBrowerTestBase: Move TestVolume

Move TestVolume helper code down, closer to where it's first used.

Bug:  833834 
Change-Id: Id2620b791e434941c28cbb308d23cd0350b3a9d2
Reviewed-on: https://chromium-review.googlesource.com/1013771
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Tomasz Mikolajewski <mtomasz@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551645}
[modify] https://crrev.com/7b2aadcb28ffc764aecba1ebfc1b1916e9da5ff8/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Comment 3 by sashab@chromium.org, Apr 19 2018

Labels: CrOSFilesCategory-CodeHealth
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2018

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

commit b902138002efcc29eb5c34558e9a8ab52cc3bbed
Author: Noel Gordon <noel@chromium.org>
Date: Fri Apr 20 02:08:29 2018

Add and correct some commentary in integration_test test_util.js

Bug:  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I12a7179f546bc3e7a6c3c12ae436e87a56adc870
Reviewed-on: https://chromium-review.googlesource.com/1019261
Commit-Queue: Luciano Pacheco (SYD) <lucmult@chromium.org>
Reviewed-by: Luciano Pacheco (SYD) <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552230}
[modify] https://crrev.com/b902138002efcc29eb5c34558e9a8ab52cc3bbed/ui/file_manager/integration_tests/test_util.js

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 20 2018

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

commit 749d1a1d26ed754b04fd6ab932fa7235aa8d0a6e
Author: Noel Gordon <noel@chromium.org>
Date: Fri Apr 20 04:56:28 2018

Clarify more commentary in integration_test test_util.js

Bug:  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I98be3602abdc4a6426680f157fa86230ef3b33a8
Reviewed-on: https://chromium-review.googlesource.com/1020781
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552264}
[modify] https://crrev.com/749d1a1d26ed754b04fd6ab932fa7235aa8d0a6e/ui/file_manager/integration_tests/test_util.js

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 21 2018

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

commit 447e96ae9f60d4e716d97385d2f4480aed2fc498
Author: Noel Gordon <noel@chromium.org>
Date: Sat Apr 21 04:44:39 2018

Tests in the testcase namespace should be anon Function

Two files in the testcase namespace do not use anon Function while all
other files do. We are all individuals, but there's little sense being
different here, it buys us nothing [1].

Fix those files (providers.js, transfer.js) to use anon Functions. Add
comments to each test. Add TODO about improving test names.

No change in behavior, no new tests.

[1] An upcoming CL will enforce that testcase namespace tests are anon
Function to make our preference for testcase code consistency clear.

Tbr: yamaguchi-san
Bug:  692034 ,  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I955c64a24b4b063f9b8eb02a3a12b3a8816ad7b3
Reviewed-on: https://chromium-review.googlesource.com/1023350
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552575}
[modify] https://crrev.com/447e96ae9f60d4e716d97385d2f4480aed2fc498/ui/file_manager/integration_tests/file_manager/providers.js
[modify] https://crrev.com/447e96ae9f60d4e716d97385d2f4480aed2fc498/ui/file_manager/integration_tests/file_manager/transfer.js

Project Member

Comment 7 by bugdroid1@chromium.org, Apr 21 2018

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

commit 6e309fbd7b10a3222a2a9cbaa3a2c34f5e3f7887
Author: Noel Gordon <noel@chromium.org>
Date: Sat Apr 21 05:51:19 2018

Fix a pre-submit problem in video_player background.js

Unneeded semi-colon trailing a named function definition.

Tbr: lucmult
Bug:  833834 
Change-Id: Ie1c2dc33b2eba75c35f974befb178058f10cf719
Reviewed-on: https://chromium-review.googlesource.com/1023430
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552579}
[modify] https://crrev.com/6e309fbd7b10a3222a2a9cbaa3a2c34f5e3f7887/ui/file_manager/integration_tests/video_player/background.js

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 21 2018

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 21 2018

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

commit 3c18a85d5515e527707def269fd027b59bdef464
Author: Noel Gordon <noel@chromium.org>
Date: Sat Apr 21 09:21:52 2018

FileManagerBrowserTest: log test case name

Rather than LOG the test extension name, LOG the test case name.

Tbr: yamaguchi-san
Bug:  692034 ,  833834 
Change-Id: I6b2cf99860fa937d4801a5d7236151d7bb013f44
Reviewed-on: https://chromium-review.googlesource.com/1023490
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552586}
[modify] https://crrev.com/3c18a85d5515e527707def269fd027b59bdef464/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 23 2018

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

commit 14bb2018097509c5f3f615af2a550fced4ad3789
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 23 02:46:06 2018

Test extensions "[Success] undefined" messages are unhelpful

The message is printed on test success in the test extension JS, since
testcase namespace test functions are anon JS Function and chrome.test
prints their Function.name property as "undefined" it seems.

  "No function in a framework trace should have a useless name. And
   the most useless name of all is: (anonymous function)."

Better to show the actual test name in the "[SUCCESS] ..." message but
a Function .name property is not writable. Is there a work-around?

Change the test extension background.js test drivers: add comments for
each step. In the step that runs the test, assert the test function is
Function type with no name (anon) [1].

Next, duct tape the desired test case name onto the test function: via
a Symbol property of an Object with a property value wrapping the test
function. Thus, Object[Symbol] is a Function with .name property equal
to the desired test case name, and a body equal to the test function.

Object[Symbol] can then be used to call it via chrome.test.RunTests so
do that. And by these semi-magical incantations, the desired test case
name (defined by the Symbol) appears: "SUCCESS test case name".

[1] See also crrev.com/552575

Bug:  692034 ,  833834 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ie3e8846df053d90722e7d7d15ddd120d758a2224
Reviewed-on: https://chromium-review.googlesource.com/1021100
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552623}
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/audio_player/background.js
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/file_manager/background.js
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/gallery/background.js
[modify] https://crrev.com/14bb2018097509c5f3f615af2a550fced4ad3789/ui/file_manager/integration_tests/video_player/background.js

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 23 2018

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

commit 2e9c078e4295c39e040a67a366626ca032ff1871
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 23 23:18:20 2018

Make FileManager browser test consistent with other browser tests

Use public. Document the browser test classes. Order the base virtuals
to match our other browser tests.

No change in behavior, no new tests.

Bug:  833834 
Change-Id: I3695fabab379d86cc6cdc38bde3c6312832c567a
Reviewed-on: https://chromium-review.googlesource.com/1023678
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552885}
[modify] https://crrev.com/2e9c078e4295c39e040a67a366626ca032ff1871/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 24 2018

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

commit 9eb73721c110b7d875a02c0d9fa57b34d7ba9956
Author: Noel Gordon <noel@chromium.org>
Date: Tue Apr 24 00:21:52 2018

Make Gallery browser test consistent with other browser tests

Use protected. Don't abbr the template parameter (use MODE).

No change in behavior, no new tests.

Bug:  833834 
Change-Id: I1fd84e01fbb22ed93b2951986afeb408c448e281
Reviewed-on: https://chromium-review.googlesource.com/1023680
Reviewed-by: Luciano Pacheco (SYD) <lucmult@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552920}
[modify] https://crrev.com/9eb73721c110b7d875a02c0d9fa57b34d7ba9956/chrome/browser/chromeos/file_manager/gallery_browsertest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 24 2018

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

commit 5557776823f064dbce4a45c9250dafdc29e7bc24
Author: Noel Gordon <noel@chromium.org>
Date: Tue Apr 24 04:20:18 2018

FileManagerBrowserTestBase: use private

Move private parts of the FileManagerBrowserTestBase API to the class
private: area.

Add TODO comments or for other work needed.

Bug:  833834 
Change-Id: I1070df30a09adbdec24dc6dfa4d0168777f92dca
Reviewed-on: https://chromium-review.googlesource.com/1025432
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552994}
[modify] https://crrev.com/5557776823f064dbce4a45c9250dafdc29e7bc24/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Project Member

Comment 14 by bugdroid1@chromium.org, Apr 24 2018

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

commit de80dbc7d677dd131824beb1f92e347415f12972
Author: Noel Gordon <noel@chromium.org>
Date: Tue Apr 24 16:14:24 2018

Minor comment corrections to FileManagerBrowserTest helpers

Comment only change.

Tbr: slangley
Bug:  833834 
Change-Id: I3e554d4fc2dd55811453601947016b49df2f63f7
Reviewed-on: https://chromium-review.googlesource.com/1025500
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553152}
[modify] https://crrev.com/de80dbc7d677dd131824beb1f92e347415f12972/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 29 2018

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

commit 07ee576c1b3e925d2762d2191c9b15b1dc9b834a
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 29 22:42:25 2018

Tweak various test volume class comments

Also change the DownloadsTestVolume Mount() code to match the way other
test volumes Mount().

No change in behavior, no new tests.

Bug:  833834 
Change-Id: Ifc31a5d2157eb605e4d9763c40bde2b0ff367bb7
Reviewed-on: https://chromium-review.googlesource.com/1034045
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554674}
[modify] https://crrev.com/07ee576c1b3e925d2762d2191c9b15b1dc9b834a/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 29 2018

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

commit ac2d543785fd391d7490f56fab605b8a73b02e43
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 29 22:46:25 2018

Make GetTestFilePath internal to TestVolume

Make helper routine GetTestFilePath internal to class TestVolume and its
derived classes, which are its only callers.

Also change its name to GetTestDataFilePath() since the file created, or
read, must reside in file manager's test data directory.

Bug:  833834 
Change-Id: I49bc321916c2a2a0fabaa1570c6a47ea890d9568
Reviewed-on: https://chromium-review.googlesource.com/1034047
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554675}
[modify] https://crrev.com/ac2d543785fd391d7490f56fab605b8a73b02e43/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Apr 29 2018

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

commit 9e2d138b77ac620fcd8dd4f41881487b91317ff0
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 29 22:48:24 2018

Document file_manager_browsertest_base.h methods

Bug:  833834 
Change-Id: I261665a70256ba03d838dddfe546dedcfd397865
Reviewed-on: https://chromium-review.googlesource.com/1034044
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554676}
[modify] https://crrev.com/9e2d138b77ac620fcd8dd4f41881487b91317ff0/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Project Member

Comment 18 by bugdroid1@chromium.org, Apr 29 2018

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

commit 455d0c4bb35063087efc7e0d6b3104528db0d1a9
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 29 23:46:47 2018

Internalize static RegisterJSONConverter helpers

Move JSON conversion helper definitions into their class so we declare
and define them once only (less to read). Document class members.

Bug:  833834 
Change-Id: Ic60e833271bce822795f63e34956992a51ad7f74
Reviewed-on: https://chromium-review.googlesource.com/1034412
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554679}
[modify] https://crrev.com/455d0c4bb35063087efc7e0d6b3104528db0d1a9/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Apr 29 2018

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

commit c07e0eb57a203e882d1aaa27edc010a3f4d137a9
Author: Noel Gordon <noel@chromium.org>
Date: Sun Apr 29 23:59:17 2018

FileManagerBrowserTestBase: use modern ctor dtor

Tbr: slangley, yamaguchi
Bug:  833834 
Change-Id: I0b1e1ad58e66e5822449e1d03b8ff7bdd9502df9
Reviewed-on: https://chromium-review.googlesource.com/1034413
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554680}
[modify] https://crrev.com/c07e0eb57a203e882d1aaa27edc010a3f4d137a9/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Apr 30 2018

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

commit 027488f663a9c166661f922c5799bcb3fc951050
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 30 00:24:17 2018

Add a comment about ownership of the drive integration pointer

Comment only change.

No-Try: true
Bug:  833834 
Change-Id: I17bfb7a9ba9c9146a1c5496001b368f00a627f55
Reviewed-on: https://chromium-review.googlesource.com/1034414
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554683}
[modify] https://crrev.com/027488f663a9c166661f922c5799bcb3fc951050/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Project Member

Comment 21 by bugdroid1@chromium.org, Apr 30 2018

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

commit a4deb11705bf0ea70fe8df30039e76f875d76823
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 30 01:18:09 2018

Internalize MapStringToTargetVolume helper in AddEntriesMessage

MapStringToTargetVolume is only used by AddEntriesMessage: move it to
AddEntriesMessage as a static helper.

Bug:  833834 
Change-Id: Iafcc7f5d2a464987145f8102c2e476ad89484243
Reviewed-on: https://chromium-review.googlesource.com/1034416
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554685}
[modify] https://crrev.com/a4deb11705bf0ea70fe8df30039e76f875d76823/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Apr 30 2018

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

commit a5868c330e4b6607d4c7607b2fc4c867e4c3f2c6
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 30 10:51:59 2018

Internalize TestEntryInfo MapString helpers

These MapString* helpers are only used by TestEntryInfo, so move them
into TextEntryInfo as internal static helpers.

Also update MapString* comments and their argument names (output does
not really work for me).

Bug:  833834 
Change-Id: Iaef563ab140df5ae1163f2bc9dff72243f78a2c5
Reviewed-on: https://chromium-review.googlesource.com/1034485
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554717}
[modify] https://crrev.com/a5868c330e4b6607d4c7607b2fc4c867e4c3f2c6/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Apr 30 2018

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

commit 4e8ee8763cd8bff8daabb852c78a6b504de1958e
Author: Noel Gordon <noel@chromium.org>
Date: Mon Apr 30 12:29:04 2018

Rename FileManagerBrowserTestBase OnMessage to OnCommand

The test extension comments say they send commands to the browser test
(FileManagerBrowserTestBase) harness. OnMessage() processes them: call
that routine OnCommand instead, to make it clear that it processes the
command messages sent by the test extensions.

No change in behavior, no new tests.

Bug:  833834 
Change-Id: Ifea9996804dbc59b434f2b3cf3e5564172e1a22a
Reviewed-on: https://chromium-review.googlesource.com/1034614
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Luciano Pacheco (SYD) <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554727}
[modify] https://crrev.com/4e8ee8763cd8bff8daabb852c78a6b504de1958e/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/4e8ee8763cd8bff8daabb852c78a6b504de1958e/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Project Member

Comment 24 by bugdroid1@chromium.org, May 1 2018

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

commit 974cda70eb73f319cfaf7e99bbbfadbccf6e7f42
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 01 00:38:47 2018

Use better variable names in RunTestMessageLoop

The message loop processes chrome.test messages: PASS, FAIL, and named
command _messages_ sent from the test extension.

Rename |entry| to |message|. Use auto where useful and add commentary.
No change in behavior, no new tests.

Tbr: slangley, yamaguchi
No-Presubmit: true
Bug:  833834 
Change-Id: Ieacf994cd1cc9fb7004ef24dd869b3ce9d288c18
Reviewed-on: https://chromium-review.googlesource.com/1034617
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554944}
[modify] https://crrev.com/974cda70eb73f319cfaf7e99bbbfadbccf6e7f42/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 25 by bugdroid1@chromium.org, May 1 2018

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

commit b25dfa70541bae1304f07eb5fad157c146c103a4
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 01 08:09:49 2018

FileMangerBrowserTests launch the test extension

InstallExtension (install has other connotations): rename this helper
to LaunchExtension() since that's what is does. Similarly, change the
virtual helper name to GetTestExtensionManifestName().

Bug:  833834 
Change-Id: Ib41febb6b2259a68ae704fd7005c2745c1a9c440
Reviewed-on: https://chromium-review.googlesource.com/1034616
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555016}
[modify] https://crrev.com/b25dfa70541bae1304f07eb5fad157c146c103a4/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc
[modify] https://crrev.com/b25dfa70541bae1304f07eb5fad157c146c103a4/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/b25dfa70541bae1304f07eb5fad157c146c103a4/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/b25dfa70541bae1304f07eb5fad157c146c103a4/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h
[modify] https://crrev.com/b25dfa70541bae1304f07eb5fad157c146c103a4/chrome/browser/chromeos/file_manager/gallery_browsertest.cc
[modify] https://crrev.com/b25dfa70541bae1304f07eb5fad157c146c103a4/chrome/browser/chromeos/file_manager/video_player_browsertest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, May 1 2018

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

commit c1a8afbfac449c71b93799af6cdaba03ef20c4b8
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 01 11:18:09 2018

getCwsWidgetContainerMockUrl command comment and style fix

The comment is clearly wrong: fix it. Use |dictionary| per style: don't
abbv varible names.

Bug:  833834 
Change-Id: I42834af7f8b4b94ba38873a671dbf50d43db9565
Reviewed-on: https://chromium-review.googlesource.com/1036984
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555025}
[modify] https://crrev.com/c1a8afbfac449c71b93799af6cdaba03ef20c4b8/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 27 by bugdroid1@chromium.org, May 1 2018

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

commit 2eb933a03a798f1bab382dce94cdb38b4a0d6b0b
Author: Noel Gordon <noel@chromium.org>
Date: Tue May 01 22:42:39 2018

Rename command installProviderExtension to launchProviderExtension

crrev.com/555016 changed the C++ browser-test-side code to use the name
LaunchExtension(), rather than InstallExtension(). Change the test code
JS command similarly, use launchProviderExtension.

Also make launchProviderExtension a separate getSetupSteps step because
chrome test.sendMessage always has a reply. We should await it and then
do the next step: setup that next step to call setupAndWaitUntilReady()
as before.

Covered by existing tests: Provider/FileManagerBrowserTest/Test.*

Bug:  833834 , 668680 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0cffa47ae94e46699fd1e92d69dea946686d6ad1
Reviewed-on: https://chromium-review.googlesource.com/1036747
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555205}
[modify] https://crrev.com/2eb933a03a798f1bab382dce94cdb38b4a0d6b0b/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/2eb933a03a798f1bab382dce94cdb38b4a0d6b0b/ui/file_manager/integration_tests/file_manager/providers.js

Project Member

Comment 28 by bugdroid1@chromium.org, May 2 2018

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

commit b082f7edc9d16ee88ee6880ccb9704196ee63372
Author: Noel Gordon <noel@chromium.org>
Date: Wed May 02 01:55:55 2018

Chromium Rule: ScopedTempDir should always be destroyed last

Many tear-down can occur if this is not the case: bug references can be
for the curious.

Tbr: slangley, yamaguchi
No-Presubmit: true
Bug:  833834 
Change-Id: I5e795f7ef5b5e28af4050c9506640d8f6df8941a
Reviewed-on: https://chromium-review.googlesource.com/1038983
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555284}
[modify] https://crrev.com/b082f7edc9d16ee88ee6880ccb9704196ee63372/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 29 by bugdroid1@chromium.org, May 2 2018

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

commit e28bf02ad9d335badb1cd5a17f6c9a959c0f45fa
Author: Noel Gordon <noel@chromium.org>
Date: Wed May 02 05:14:13 2018

Make the getRootPaths command fit for human consumption

Auto formatting and line length limits can produce the unreadable. In a
business where code is read way more than it is writ, making a reader's
eye dance left, right, and back again is tiresome and unnecessary.

Re-write this code for humans: it is both shorter and clearer.

Bug:  833834 
Change-Id: I64535fdf6b4cccbc02029d594543d6719fe3c79f
Reviewed-on: https://chromium-review.googlesource.com/1039164
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555313}
[modify] https://crrev.com/e28bf02ad9d335badb1cd5a17f6c9a959c0f45fa/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 30 by bugdroid1@chromium.org, May 3 2018

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

commit 7c003f442309fcfab63ef01d6cc75d7ad1f01fb5
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 03 00:09:38 2018

Initialize FileManagerBrowserTest DriveTestVolume members

Add comment about class DriveTestVolume pointer member ownership (none
are owned). Initialize these members.

Add EXPECT cases when creating the test Drive integration service. Use
nullptr in place of NULL.

Covered by existing tests: Transfer/FileManagerBrowserTest.Test*

Bug:  833834 
Change-Id: I04d132e36d8293627f388469138ee7c58e453538
Reviewed-on: https://chromium-review.googlesource.com/1039211
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555620}
[modify] https://crrev.com/7c003f442309fcfab63ef01d6cc75d7ad1f01fb5/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 31 by bugdroid1@chromium.org, May 3 2018

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

commit 8851dc48d7cc716bb2dbac0b45acf027078b774b
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 03 22:45:27 2018

Minor clickNotificationButton command code re-org

Minor clickNotificationButton re-org while I was running my ruler over
this code. No change in behavior, no new tests.

Tbr: slangley
Bug:  833834 
Change-Id: Ic23e13e89ab0d3a30d17d456c6c12c781fbc02f4
Reviewed-on: https://chromium-review.googlesource.com/1041368
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555893}
[modify] https://crrev.com/8851dc48d7cc716bb2dbac0b45acf027078b774b/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 32 by bugdroid1@chromium.org, May 3 2018

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

commit 06f88a7d833ee6b60b799e036b02e7713e240892
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 03 23:10:11 2018

White space DriveTestVolume CreateDirectory/CreateFile

Minor: the code is similar but was not formatted the same way. Make it
formatted the same way for the reader's sake.

Bug:  833834 
Change-Id: I68f8b3b46f7d02c988e59887d1dc30f1284cbb34
Reviewed-on: https://chromium-review.googlesource.com/1042325
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555896}
[modify] https://crrev.com/06f88a7d833ee6b60b799e036b02e7713e240892/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 33 by bugdroid1@chromium.org, May 3 2018

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

commit 45f1cc8fc585603dac79b02506921c193583e646
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 03 23:11:54 2018

Update FakeTestVolume comments and add a private GetMountPoints helper

Bring the FakeTestVolume comments up-to-date. Add a private helper for
getting the external mount points to make Mount() simpler.

FakeTestVolume could be used for testing read-only volumes, but is not
currently used for that: make read-only as const bool member for now.

Bug:  833834 
Change-Id: I4da213998e224d3b469c8221f6902fb3f8221e25
Reviewed-on: https://chromium-review.googlesource.com/1042425
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555899}
[modify] https://crrev.com/45f1cc8fc585603dac79b02506921c193583e646/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 34 by bugdroid1@chromium.org, May 3 2018

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

commit 8057899c3f0e3da754d6fb635e5cf9fe9f0e9306
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 03 23:13:32 2018

Commands mountFakeUsb and mountFakeMtp: assert their Mount() works

These commands create then mount fake USB and MTP volumes for testing.
Add ASSERTs that Mount() calls on these fake volumes succeeds in test.

No change in behavior, no new tests.

Bug:  833834 
Change-Id: I49c9a21b629cdb6285982d622e1020b6c16dee73
Reviewed-on: https://chromium-review.googlesource.com/1042005
Reviewed-by: Stuart Langley <slangley@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555900}
[modify] https://crrev.com/8057899c3f0e3da754d6fb635e5cf9fe9f0e9306/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 35 by bugdroid1@chromium.org, May 3 2018

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

commit 597a899086d4baebb543eb55b7bd283a3b2068a1
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 03 23:39:11 2018

Remove Param postfix from the browser test virtual state getters

FileManagerBrowserTest is a parametric test: none of the other browser
tests are (Gallery, VideoPlayer, AudioPlayer).

Having the Param postfix in the getter names is a leftover from a time
when there was only FileManagerBrowserTest. Remove the Param postfix.

Bonus points: the implementation details of the test extension browser
tests is no longer exposed. FileManageBrowserTestBase does not need to
know, nor does it care.

Bug:  833834 
Change-Id: I3919b3751122afdccd1f6f0689107ab91440c427
Reviewed-on: https://chromium-review.googlesource.com/1042426
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555907}
[modify] https://crrev.com/597a899086d4baebb543eb55b7bd283a3b2068a1/chrome/browser/chromeos/file_manager/audio_player_browsertest.cc
[modify] https://crrev.com/597a899086d4baebb543eb55b7bd283a3b2068a1/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc
[modify] https://crrev.com/597a899086d4baebb543eb55b7bd283a3b2068a1/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/597a899086d4baebb543eb55b7bd283a3b2068a1/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h
[modify] https://crrev.com/597a899086d4baebb543eb55b7bd283a3b2068a1/chrome/browser/chromeos/file_manager/gallery_browsertest.cc
[modify] https://crrev.com/597a899086d4baebb543eb55b7bd283a3b2068a1/chrome/browser/chromeos/file_manager/video_player_browsertest.cc

Project Member

Comment 36 by bugdroid1@chromium.org, May 4 2018

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

commit d199686577e902744e738cbbd74b606c8497505f
Author: Noel Gordon <noel@chromium.org>
Date: Fri May 04 00:25:52 2018

Add AddEntriesMessage::ConvertJSONValue helper

Add a AddEntriesMessage helper to convert the JSON dictionary value of
the addEntries command to an AddEntriesMessage |message|.

In the addEntries command, use that converter to extract the |message|
and also FAIL if the message.volume is an invalid volume type.

Covered by existing FileManager browser tests.

Bug:  833834 
Change-Id: Ib7ec56fee7c5b4857ccdfb0155a414430db73e97
Reviewed-on: https://chromium-review.googlesource.com/1042125
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555922}
[modify] https://crrev.com/d199686577e902744e738cbbd74b606c8497505f/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 37 by bugdroid1@chromium.org, May 4 2018

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

commit 1f74b1a070598d9f1d5095f7a418960d3fa39cde
Author: Noel Gordon <noel@chromium.org>
Date: Fri May 04 00:43:03 2018

Follow-up crrev.com/555893 use std::string instead of auto

Bug:  833834 
Change-Id: Iea9d32052c6ffbe17bf2cdd5075cbf5e9a203e0a
Reviewed-on: https://chromium-review.googlesource.com/1042428
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555932}
[modify] https://crrev.com/1f74b1a070598d9f1d5095f7a418960d3fa39cde/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Project Member

Comment 38 by bugdroid1@chromium.org, May 6 2018

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

commit fa744efac3f841d8be912acdffdbf550d37e2460
Author: Noel Gordon <noel@chromium.org>
Date: Sun May 06 23:32:32 2018

<linked_ptr> is deprecated

Bug:  833834 
Change-Id: I4eb15bf48c996adad73f72597718b82af1069654
Reviewed-on: https://chromium-review.googlesource.com/1045948
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556361}
[modify] https://crrev.com/fa744efac3f841d8be912acdffdbf550d37e2460/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/fa744efac3f841d8be912acdffdbf550d37e2460/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Project Member

Comment 39 by bugdroid1@chromium.org, May 7 2018

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

commit 6a73e55948ca2b2ea3e3e643e57b28dc9d109d29
Author: Noel Gordon <noel@chromium.org>
Date: Mon May 07 00:21:32 2018

Clarify the "isInGuestMode" test extension command

Add FileManageBrowserTestBase private helper routines: IsGuestModeTest
and IsIncognitoModeTest to return true when the test extension browser
test has that GetGuestMode(). Use the helpers everywhere.

The "isInGuestMode" command actually wants to know if the test runs in
guest or incognito mode, or not. Write that out explicitly in its code
and add LOG(INFO) and ASSERT_EQ() for even more explanation.

Move "isInGuestMode" command so it's the first, move "getTestCaseName"
command so it's third, to match the call order the test extensions use
to call them: isInGuestMode, getRootPaths, getTestCaseName [1].

Minor change to getRootPaths: call the variable downloads_root.

[1] The test extensions call in this order always and once these three
are done, the test extensions start running the test case. Another way
to think about it is: these three commands are preliminary set-up that
every Files.app browser test case requires. All remaining commands are
specific to the current test case.

Bug:  833834 
Change-Id: Ida01b88e745df6bf4b760aa421d36c5d8e872ecd
Reviewed-on: https://chromium-review.googlesource.com/1043672
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556362}
[modify] https://crrev.com/6a73e55948ca2b2ea3e3e643e57b28dc9d109d29/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc
[modify] https://crrev.com/6a73e55948ca2b2ea3e3e643e57b28dc9d109d29/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.h

Project Member

Comment 40 by bugdroid1@chromium.org, May 7 2018

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

commit d4901701499c33ae59f022eb898120217c019b13
Author: Noel Gordon <noel@chromium.org>
Date: Mon May 07 04:43:30 2018

Introduce the AddEntriesMessage concept first

Put AddEntriesMessage first in the code to introduce the concept, move
TestEntryInfo afterward (it's just a helper). Run 'git cl format' over
this file to format the enum's per style.

No change in behavior, no new tests.

Tbr: slangley, yamaguchi-san
Bug:  833834 
Change-Id: I4ea9333b090e4beead7217c843d3416c927ed933
Reviewed-on: https://chromium-review.googlesource.com/1045959
Reviewed-by: Noel Gordon <noel@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556377}
[modify] https://crrev.com/d4901701499c33ae59f022eb898120217c019b13/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Comment 41 by noel@chromium.org, May 7 2018

Cc: slangley@chromium.org
Status: Fixed (was: Assigned)
That was fun, but that's all I wanted to do here folks.  Thanks for all the reviews @slangley.

Comment 43 by noel@chromium.org, May 10 2018

Status: Started (was: Fixed)
Couple of DISALLOW_COPY_AND_ASSIGN style fixes #42.  Looking at one more.

Comment 44 Deleted

Project Member

Comment 45 by bugdroid1@chromium.org, May 10 2018

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

commit 77f2d632ce4b5a214be4849666cf60ad167877aa
Author: Noel Gordon <noel@chromium.org>
Date: Thu May 10 07:04:11 2018

Make FileManagerBrowserTestBase inner classes DISALLOW_COPY_AND_ASSIGN

Bug:  833834 
Change-Id: I208207b46af243ce36fc810363b3e727eee41a2d
Reviewed-on: https://chromium-review.googlesource.com/1053331
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Stuart Langley <slangley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557471}
[modify] https://crrev.com/77f2d632ce4b5a214be4849666cf60ad167877aa/chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc

Comment 46 Deleted

Comment 47 by noel@chromium.org, May 10 2018

Fixed.  For clang tidy, see issue 841659.

Sign in to add a comment