Stop foreground depending on background in gyp v2

Stop foreground depending on background in gyp v2

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2883263002
Cr-Commit-Position: refs/heads/master@{#472458}
Committed: https://chromium.googlesource.com/chromium/src/+/27a05b28ecc1e505a282463337a36d09011b947d

[oka, 2017-05-16 14:41:16]

Description was changed from

==========
Stop foreground depending on background.

BUG=636289
TEST=run_compiler
==========

to

==========
Stop foreground depending on background.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[oka, 2017-05-16 14:42:10]

fix.

[oka, 2017-05-16 15:19:23]

.

[oka, 2017-05-16 15:22:56]

sort

[oka, 2017-05-16 15:30:45]

Remove private

[oka, 2017-05-16 15:32:13]

oka@chromium.org changed reviewers:
+ fukino@chromium.org

[oka, 2017-05-16 15:32:13]

PTAL.

[oka, 2017-05-16 15:32:43]

Description was changed from

==========
Stop foreground depending on background.

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Stop foreground depending on background in gyp v2

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[fukino, 2017-05-17 04:58:56]

Thank you for working on this!

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
File ui/file_manager/externs/background/media_scanner.js (right):

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:12: importer.MediaScanner =
function() {};
Can we remove duplicated definition of importer.MediaScanner from
ui/file_manager/file_manager/background/js/media_scanner.js?

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:69:
importer.DefaultMediaScanner = function(
Do we need importer.DefaultMediaScanner in foreground?

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:90: importer.ScanResult =
function() {};
Can we remove duplicated definition from
ui/file_manager/file_manager/background/js/media_scanner.js?

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:192:
importer.DefaultScanResult = function(mode, hashGenerator) {};
importer.DefaultScanResult is not used from foreground, so we won't need it in
externs.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:254:
importer.DirectoryWatcher = function() {};
ditto

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:280:
importer.DefaultDirectoryWatcher = function(callback) {};
ditto

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
File ui/file_manager/externs/background/task_queue.js (right):

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/task_queue.js:21: importer.TaskQueue =
function() {};
Do we have to expose full shape of importer.TaskQueue and
importer.MediaImportHandler.ImportTask?
It'd be great if we can extract the minimum interface which the foreground need
to know.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
File ui/file_manager/externs/background/volume_manager_factory.js (right):

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/volume_manager_factory.js:4: 
Do we need this file in addition to background_window_common.js?

[oka, 2017-05-17 08:44:15]

Addressed comments.

[oka, 2017-05-17 08:45:42]

PTAL.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
File ui/file_manager/externs/background/media_scanner.js (right):

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:12: importer.MediaScanner =
function() {};
On 2017/05/17 04:58:56, fukino wrote:
> Can we remove duplicated definition of importer.MediaScanner from
> ui/file_manager/file_manager/background/js/media_scanner.js?

Done.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:69:
importer.DefaultMediaScanner = function(
On 2017/05/17 04:58:56, fukino wrote:
> Do we need importer.DefaultMediaScanner in foreground?

Removed.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:90: importer.ScanResult =
function() {};
On 2017/05/17 04:58:55, fukino wrote:
> Can we remove duplicated definition from
> ui/file_manager/file_manager/background/js/media_scanner.js?

Done.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:192:
importer.DefaultScanResult = function(mode, hashGenerator) {};
On 2017/05/17 04:58:56, fukino wrote:
> importer.DefaultScanResult is not used from foreground, so we won't need it in
> externs.

Done.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:254:
importer.DirectoryWatcher = function() {};
On 2017/05/17 04:58:56, fukino wrote:
> ditto

Done.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/media_scanner.js:280:
importer.DefaultDirectoryWatcher = function(callback) {};
On 2017/05/17 04:58:56, fukino wrote:
> ditto

Done.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
File ui/file_manager/externs/background/task_queue.js (right):

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/task_queue.js:21: importer.TaskQueue =
function() {};
On 2017/05/17 04:58:56, fukino wrote:
> Do we have to expose full shape of importer.TaskQueue and
> importer.MediaImportHandler.ImportTask?
> It'd be great if we can extract the minimum interface which the foreground
need
> to know.

Removed needless exposure.

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
File ui/file_manager/externs/background/volume_manager_factory.js (right):

https://codereview.chromium.org/2883263002/diff/80001/ui/file_manager/externs...
ui/file_manager/externs/background/volume_manager_factory.js:4: 
On 2017/05/17 04:58:56, fukino wrote:
> Do we need this file in addition to background_window_common.js?

Good catch. Just renamed background_window_common.js.

[oka, 2017-05-17 09:50:43]

Removed unused TaskQueue and values in media_import_handler.

[oka, 2017-05-17 09:50:57]

PTAL.

[fukino, 2017-05-17 10:49:19]

lgtm with nits.

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
File ui/file_manager/externs/background/media_import_handler.js (right):

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
ui/file_manager/externs/background/media_import_handler.js:13:
importer.ImportRunner = function() {};
Could you add an extern file for importer.ImportRunner, and remove the interface
definition from
ui/file_manager/file_manager/background/js/media_import_handler.js?

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
File ui/file_manager/externs/background/volume_manager_factory.js (right):

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
ui/file_manager/externs/background/volume_manager_factory.js:1: // Copyright
2017 The Chromium Authors. All rights reserved.
nit: You don't have to update the year when moving files.

[oka, 2017-05-17 14:16:15]

Addressed comments. Extracted ImportRunner interface as a file

[oka, 2017-05-17 14:38:36]

Forgot to add import_runner.js

[oka, 2017-05-17 14:39:29]

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
File ui/file_manager/externs/background/media_import_handler.js (right):

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
ui/file_manager/externs/background/media_import_handler.js:13:
importer.ImportRunner = function() {};
On 2017/05/17 10:49:19, fukino wrote:
> Could you add an extern file for importer.ImportRunner, and remove the
interface
> definition from
> ui/file_manager/file_manager/background/js/media_import_handler.js?

Done.

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
File ui/file_manager/externs/background/volume_manager_factory.js (right):

https://codereview.chromium.org/2883263002/diff/120001/ui/file_manager/extern...
ui/file_manager/externs/background/volume_manager_factory.js:1: // Copyright
2017 The Chromium Authors. All rights reserved.
On 2017/05/17 10:49:19, fukino wrote:
> nit: You don't have to update the year when moving files.

Done.

[oka, 2017-05-17 14:39:52]

Format.

[oka, 2017-05-17 14:40:08]

The CQ bit was checked by oka@chromium.org

[oka, 2017-05-17 14:40:09]

The patchset sent to the CQ was uploaded after l-g-t-m from fukino@chromium.org
Link to the patchset: https://codereview.chromium.org/2883263002/#ps180001
(title: "Format.")

[commit-bot: I haz the power, 2017-05-17 14:40:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-17 15:18:30]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1495032008680240,
"parent_rev": "2701c48359353889f90c30309ff523273890cca7", "commit_rev":
"27a05b28ecc1e505a282463337a36d09011b947d"}

[commit-bot: I haz the power, 2017-05-17 15:18:44]

Description was changed from

==========
Stop foreground depending on background in gyp v2

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Stop foreground depending on background in gyp v2

BUG=636289
TEST=run_compiler
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2883263002
Cr-Commit-Position: refs/heads/master@{#472458}
Committed:
https://chromium.googlesource.com/chromium/src/+/27a05b28ecc1e505a282463337a3...
==========

[commit-bot: I haz the power, 2017-05-17 15:18:45]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/27a05b28ecc1e505a282463337a3...