fukino@, can you provide more details about what the issues were?  If it's a problem with the generator, we should fix it, or if the existing code that references the externs needs to be updated, we should do that.  The more we can move to having these be generated, the better.

Let me check the current status.

I made a CL to use the generated one in file_manager.(https://codereview.chromium.org/2640673002/)
file_manager_private.js is generated by
tools/json_schema_compiler/compiler.py -g externs chrome/common/extensions/api/file_manager_private.idl

I need a few modifications on the generated one. The diff is
https://codereview.chromium.org/2640673002/diff2/1:20001/third_party/closure_compiler/externs/file_manager_private.js
For example, chrome.fileManagerPrivate API refers a type which is defined in chrome.fileSystemProvider API.
It is referred as fileSystemProvider.Action in
https://codesearch.chromium.org/chromium/src/chrome/common/extensions/api/file_manager_private.idl?q=chrome/common/extensions/api/file_manager_private.idl&sq=package:chromium&dr&l=659
However, in the generated externs file, it is referred as chrome.fileManagerPrivate.fileSystemProvider.Action. It is intended to be chrome.fileSystemProvider.Action.

Additional nice-to-have feature for the generator are:
- Generated externs have @see links to online documents, but private APIs do not have such online documents. It'd be great if we have an option to omit @see links.
- We have two empty lines between top-level blocks in chrome_extensions.js, but in generated externs we have one empty lines. Having two empty lines will make it a bit easier to copy generated definitions to chrome_extensions.js

updating this massive externs file manually is definitely a pain.

For a break from UI - this is a great high-impact starter bug that would help the team a lot! :)

The externs file is generated whenever we edit the file_manager_private.idl file (our API for the files app). To try it, edit something in file_manager_private.idl, then commit and run git cl upload. You should get a warning saying to run the generator... But if you do that, you'll see it generates an incomplete js file that is different from the (currently hand-written) js file we write. This bug involves fixing the generator so we don't have to write it manually :)

If I edit the .idl file, the .js file or both, how do I tell whether I broke anything?

For example, I deleted a bunch of lines from the .js file, and "ninja -C out/foo chrome" still seems happy. I ran out/foo/chrome (built with target_os = "chromeos"), clicked on a few things in the Files app, and nothing seemed obviously broken.

For example, while https://chromium-review.googlesource.com/c/chromium/src/+/1065834 edited both the .idl and the .js files, https://chromium-review.googlesource.com/c/chromium/src/+/1067378 and https://chromium-review.googlesource.com/c/chromium/src/+/1056968 are recent CLs that edited only the .idl file.

Maybe the closure compiler might include the JS file and detect if things are broken.

 ninja -C out/foo ui/file_manager:closure_compile

Yes, that squeals if I mess with the .js file.

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

commit dd2af2ca458e410d29907189b09db0c1f79eb577
Author: Nigel Tao <nigeltao@chromium.org>
Date: Wed Jun 06 04:45:12 2018

Re-order file_manager_private.js declarations

This neither adds nor deletes declarations. It reorders them to lessen
the diff on a subsequent commit (which automatically generates
file_manager_private.js by running
tools/json_schema_compiler/compiler.py on file_manager_private.idl).

Two declarations (EntryAction, installWebstoreItem) don't have any
corresponding entry in the file_manager_private.idl file.

Test: "ninja ui/file_manager:closure_compile" runs without error
Bug: 613096
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I6dcc4a8f4f51182f7e33509decebe72b221f3ea6
Reviewed-on: https://chromium-review.googlesource.com/1082185
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564789}
[modify] https://crrev.com/dd2af2ca458e410d29907189b09db0c1f79eb577/third_party/closure_compiler/externs/file_manager_private.js

BTW, chrome/common/extensions/api/file_manager_private.idl has this line:

callback GetCustomActionsCallback = void(fileSystemProvider.Action[] actions);

Note that the element type name is qualified: "fileSystemProvider.Action" and not just "Action". Indeed, there is an Action type defined in chrome/common/extensions/api/file_system_provider.idl. Note that the file names are f_s_p.idl and f_m_p.idl.

The hand-written JS in third_party/closure_compiler/externs/file_manager_private.js has this:

@param {function((!Array<!EntryAction>|undefined))} callback

where EntryAction is defined in the hand-written .js file but not the canonical .idl file:

/**
 * @typedef {{
 *   id: string,
 *   title: (string|undefined)
 * }}
 */
var EntryAction;

Now, I'd like to replace !Array<!EntryAction> in the JS with !Array<!chrome.fileSystemProvider.Action>, but... there is no file_system_provider.js file under third_party/closure_compiler/externs/, only a file_manager_private.js file. Again, note that the file names are f_s_p.js and f_m_p.js.

$ ls third_party/closure_compiler/externs/file_*
third_party/closure_compiler/externs/file_manager_private.js

What should I do? I could add configuration to also generate f_s_p.js, but does that have security implications? Is that fundamentally working against the grain of the closure compiler?

Alternatively, I could add an Action typedef to (or re-name and re-purpose the EntryAction typedef in) the f_m_p.idl file, to mimic the f_s_p.idl Action:

dictionary Action {
  // The identifier of the action. Any string or $(ref:CommonActionId) for
  // common actions.
  DOMString id;

  // The title of the action. It may be ignored for common actions.
  DOMString? title;
};

but that would only be a copy/paste and could diverge over time, so it doesn't seem like the right approach.

Any suggestions?

I think you should just generate f_s_p.idl.

What are the security concerns you have? I'm not sure how Closure externs would affect security. The Closure compiler's grain is very much 'Generate externs for everything that is defined externally'.

I assume calmity@ mean't generate f_s_p.js. +1 to that.

We have other examples where we reference data structures in other idls, but only one place I know of where that produces an external reference in a generated extern: In bluetooth_private.js we reference chrome.bluetooth.Device, but we do a replacement by hand (which has a comment in the header): 

// s/chrome.bluetoothPrivate.bluetooth.Device/chrome.bluetooth.Device/

It would be fantastic to make that 'just work', but the extra step isn't a very big deal and is much easier than maintaining the externs by hand.

I also agree that there should not be any security concerns with generated externs. The JS in the generated externs should never be executed, only used for linting.

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

commit 7a8e958866ae0e1514e9ec78223caf13e7ef29c3
Author: Nigel Tao <nigeltao@chromium.org>
Date: Wed Jun 06 23:30:31 2018

Namespace file_manager_private.js declarations

This brings the (still hand-written) externs/file_manager_private.js
file closer to what tools/json_schema_compiler/compiler.py generates.

Test: "ninja ui/file_manager:closure_compile" runs without error
Bug: 613096
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib3dfdddd9a6b9c0d67d143f89a5f9348eaecb4ae
Reviewed-on: https://chromium-review.googlesource.com/1088345
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565095}
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/third_party/closure_compiler/externs/file_manager_private.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/externs/volume_info.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/device_handler.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/drive_sync_handler.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/import_history.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/media_scanner.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/volume_info_impl.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/volume_manager_impl.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/background/js/volume_manager_util.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/common/js/util.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/file_watcher.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/import_controller.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/metadata/external_metadata_provider.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/providers_model.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/quick_view_controller.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/ui/gear_menu.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/ui/providers_menu.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/foreground/js/ui/search_box.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/file_manager/test/js/chrome_file_manager.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/gallery/js/entry_list_watcher.js
[modify] https://crrev.com/7a8e958866ae0e1514e9ec78223caf13e7ef29c3/ui/file_manager/gallery/js/slide_mode.js

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

commit 472cb281b06ee8f24aa11f00b5f236bd11d0efcc
Author: Nigel Tao <nigeltao@chromium.org>
Date: Thu Jun 07 03:40:16 2018

Add more file_manager_private.js declarations

This brings the (still hand-written) externs/file_manager_private.js
file closer to what tools/json_schema_compiler/compiler.py generates.

Test: "ninja ui/file_manager:closure_compile" runs without error
Bug: 613096
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I18ade165986a47b1fd9e182f4f57c8f8c2b09a00
Reviewed-on: https://chromium-review.googlesource.com/1089598
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565168}
[modify] https://crrev.com/472cb281b06ee8f24aa11f00b5f236bd11d0efcc/third_party/closure_compiler/externs/file_manager_private.js

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

commit 172be4b9edff0ef700e1b57d11452f331d9ba0bf
Author: Nigel Tao <nigeltao@chromium.org>
Date: Fri Jun 08 00:20:34 2018

Generate externs/file_system_provider.js

Command line:

python tools/json_schema_compiler/compiler.py \
  chrome/common/extensions/api/file_system_provider.idl \
  --root=. --generator=externs \
  > third_party/closure_compiler/externs/file_system_provider.js

Bug: 613096
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I146dd9125ee575e7ea6592cf6805a5f9eec194b7
Reviewed-on: https://chromium-review.googlesource.com/1089595
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565481}
[modify] https://crrev.com/172be4b9edff0ef700e1b57d11452f331d9ba0bf/chrome/common/extensions/api/PRESUBMIT.py
[add] https://crrev.com/172be4b9edff0ef700e1b57d11452f331d9ba0bf/third_party/closure_compiler/externs/file_system_provider.js

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

commit bf42268f6639e4322eb56507739a33ffcbb7b28b
Author: Nigel Tao <nigeltao@chromium.org>
Date: Fri Jun 08 04:52:51 2018

Remove unused installWebstoreItem declaration

It (chrome.fileManagerPrivate.installWebstoreItem) seems superseded by
chrome.webstoreWidgetPrivate.installWebstoreItem (note the middle term
changed from fMP to wWP), defined in
ui/file_manager/externs/chrome_webstore_widget_private.js

Test: "ninja ui/file_manager:closure_compile" runs without error
Bug: 613096
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I5ca1049baef3a4cf7a66170092179bfef0af4c49
Reviewed-on: https://chromium-review.googlesource.com/1089593
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565536}
[modify] https://crrev.com/bf42268f6639e4322eb56507739a33ffcbb7b28b/third_party/closure_compiler/externs/file_manager_private.js

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

commit 63b704430bbcbed07d5138feb6a9f12606d6fb5e
Author: Nigel Tao <nigeltao@chromium.org>
Date: Wed Jun 20 02:41:57 2018

Don't generate @see links for private externs

See https://bugs.chromium.org/p/chromium/issues/detail?id=613096#c5 for
rationale

These two files:
third_party/closure_compiler/externs/accessibility_private.js
third_party/closure_compiler/externs/autofill_private.js
were updated in this commit, as an example of the change.

A follow-up commit will update other *_private.js files.

Bug: 613096
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ica4ab471956052b37c8bb6a9aabe4a7502408cfb
Reviewed-on: https://chromium-review.googlesource.com/1090425
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568702}
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/third_party/closure_compiler/externs/accessibility_private.js
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/third_party/closure_compiler/externs/autofill_private.js
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/tools/json_schema_compiler/js_externs_generator.py
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/tools/json_schema_compiler/js_externs_generator_test.py
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/tools/json_schema_compiler/js_interface_generator.py
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/tools/json_schema_compiler/js_interface_generator_test.py
[modify] https://crrev.com/63b704430bbcbed07d5138feb6a9f12606d6fb5e/tools/json_schema_compiler/js_util.py

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

commit 2e387cb34ee7a07592cb088c01b4ea48f9a9713f
Author: Nigel Tao <nigeltao@chromium.org>
Date: Wed Jun 20 07:45:39 2018

Replace EntryAction with chrome.fsp.Action

The EntryAction type is (now) fictional, and no longer corresponds to
anything in chrome/common/extensions/api/file_manager_private.idl

Test: "ninja ui/file_manager:closure_compile" runs without error
Bug: 613096
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I3afc580cbcf259fac2a450f24cb7088164ee235c
Reviewed-on: https://chromium-review.googlesource.com/1107027
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568759}
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/third_party/closure_compiler/externs/file_manager_private.js
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/audio_player/js/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/file_manager/background/js/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/file_manager/common/js/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/file_manager/foreground/js/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/file_manager/foreground/js/metadata/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/gallery/js/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/gallery/js/image_editor/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/image_loader/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/video_player/js/BUILD.gn
[modify] https://crrev.com/2e387cb34ee7a07592cb088c01b4ea48f9a9713f/ui/file_manager/video_player/js/cast/BUILD.gn