[MD settings] closure compilation

[MD settings] closure compilation

This CL adds closure compilation settings for several directories within
MD settings.

BUG=425627
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/838a2edbbdf1d7b6364c3347a61622cf987ec637
Cr-Commit-Position: refs/heads/master@{#403374}

[dschuyler, 2016-06-30 20:21:27]

Description was changed from

==========
[MD settings] closure compilation

This CL adds closure compilation settings for several directories within
MD settings.

BUG=425627
==========

to

==========
[MD settings] closure compilation

This CL adds closure compilation settings for several directories within
MD settings.

BUG=425627
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[dschuyler, 2016-06-30 20:55:08]

dschuyler@chromium.org changed reviewers:
+ tommycli@chromium.org

[dschuyler, 2016-06-30 20:55:09]

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/default_browser_page/default_browser_page.js
(left):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/default_browser_page/default_browser_page.js:58:
setAsDefaultConcluded: function() {
This is old and unused.

[tommycli, 2016-06-30 21:29:43]

lgtm sans below nits:

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_browser_proxy.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_browser_proxy.js:44:
ClearBrowsingDataBrowserProxy: ClearBrowsingDataBrowserProxy,
No need to export this symbol

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js:62:
/** @private {settings.ClearBrowsingDataBrowserProxy} */
If this type is changed to ClearBrowsingDataBrowserProxyImpl instead

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engine_dialog.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engine_dialog.js:36:
/** @private {settings.SearchEnginesBrowserProxy} */
Same here, if the type is changed to the Impl type we don't need to export the
base class

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engine_entry.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engine_entry.js:27:
/** @private {settings.SearchEnginesBrowserProxy} */
and here

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/search_engines_page/search_engines_browser_proxy.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/search_engines_page/search_engines_browser_proxy.js:139:
SearchEnginesBrowserProxy: SearchEnginesBrowserProxy,
no need

[dschuyler, 2016-06-30 22:21:15]

On 2016/06/30 21:29:43, tommycli wrote:
> lgtm sans below nits:
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
> File
>
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_browser_proxy.js
> (right):
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
>
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_browser_proxy.js:44:
> ClearBrowsingDataBrowserProxy: ClearBrowsingDataBrowserProxy,
> No need to export this symbol
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
> File
>
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js
> (right):
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
>
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js:62:
> /** @private {settings.ClearBrowsingDataBrowserProxy} */
> If this type is changed to ClearBrowsingDataBrowserProxyImpl instead
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
> File
> chrome/browser/resources/settings/search_engines_page/search_engine_dialog.js
> (right):
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
>
chrome/browser/resources/settings/search_engines_page/search_engine_dialog.js:36:
> /** @private {settings.SearchEnginesBrowserProxy} */
> Same here, if the type is changed to the Impl type we don't need to export the
> base class
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
> File
> chrome/browser/resources/settings/search_engines_page/search_engine_entry.js
> (right):
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
>
chrome/browser/resources/settings/search_engines_page/search_engine_entry.js:27:
> /** @private {settings.SearchEnginesBrowserProxy} */
> and here
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
> File
>
chrome/browser/resources/settings/search_engines_page/search_engines_browser_proxy.js
> (right):
> 
>
https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
>
chrome/browser/resources/settings/search_engines_page/search_engines_browser_proxy.js:139:
> SearchEnginesBrowserProxy: SearchEnginesBrowserProxy,
> no need

We discussed the nits offline and decided to export and use the interfaces
rather than the impl defs.

[dschuyler, 2016-06-30 22:21:20]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2016-06-30 22:21:44]

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, 2016-06-30 22:29:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-30 22:29:37]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  closure_compilation on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/closure_compila...)

[Dan Beam, 2016-06-30 22:35:33]

fyi: we're encouraged as reviewers not to repeat similar feedback over and over
across multiple files

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_browser_proxy.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_browser_proxy.js:44:
ClearBrowsingDataBrowserProxy: ClearBrowsingDataBrowserProxy,
On 2016/06/30 21:29:42, tommycli wrote:
> No need to export this symbol

you need to if you'd like to use this as a type

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
File
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js
(right):

https://codereview.chromium.org/2112033002/diff/1/chrome/browser/resources/se...
chrome/browser/resources/settings/clear_browsing_data_dialog/clear_browsing_data_dialog.js:62:
/** @private {settings.ClearBrowsingDataBrowserProxy} */
On 2016/06/30 21:29:42, tommycli wrote:
> If this type is changed to ClearBrowsingDataBrowserProxyImpl instead

that's true, but kind of defeats the purpose of an interface

[dschuyler, 2016-07-01 00:32:45]

The CQ bit was checked by dschuyler@chromium.org

[dschuyler, 2016-07-01 00:32:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
tommycli@chromium.org
Link to the patchset: https://codereview.chromium.org/2112033002/#ps20001
(title: "merge with master")

[commit-bot: I haz the power, 2016-07-01 00:33:00]

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, 2016-07-01 00:39:16]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-01 00:39:18]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-01 00:41:59]

Description was changed from

==========
[MD settings] closure compilation

This CL adds closure compilation settings for several directories within
MD settings.

BUG=425627
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD settings] closure compilation

This CL adds closure compilation settings for several directories within
MD settings.

BUG=425627
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/838a2edbbdf1d7b6364c3347a61622cf987ec637
Cr-Commit-Position: refs/heads/master@{#403374}
==========

[commit-bot: I haz the power, 2016-07-01 00:42:00]

Patchset 2 (id:??) landed as
https://crrev.com/838a2edbbdf1d7b6364c3347a61622cf987ec637
Cr-Commit-Position: refs/heads/master@{#403374}