[MD settings] certificate manager closure compilation

[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

Committed: https://crrev.com/d369e7530e080bad5679321998ea1f072b5f7d4d
Cr-Commit-Position: refs/heads/master@{#403293}

[dschuyler, 2016-06-29 22:10:32]

Description was changed from

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

BUG=None
==========

to

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

[dschuyler, 2016-06-29 22:18:43]

Patchset #1 (id:1) has been deleted

[dschuyler, 2016-06-29 22:19:47]

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

[dpapad, 2016-06-29 22:28:42]

Also need to updated
https://cs.chromium.org/chromium/src/chrome/test/data/webui/settings/certific...
accordingly.

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js:57:
* @param {*} error
This error should really be one of the two types. Is there a way to restore this
and keep the compiler happy?

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificates_browser_proxy.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificates_browser_proxy.js:18:
var Certificate;
Some of the typedefs are moved to the new file and some stayed here. Is there a
rule about which should go in the new file?

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js:19:
this.$.dialog.open();
I am surprised that this compiles. How does the compiler what is the type of
this.$.dialog? Even if it does somehow, cr-dialog is not in the deps list.

[dschuyler, 2016-06-30 00:18:06]

Description was changed from

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

to

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

[dschuyler, 2016-06-30 00:53:14]

Patchset #2 (id:40001) has been deleted

[dschuyler, 2016-06-30 17:13:32]

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificate_list.js:57:
* @param {*} error
On 2016/06/29 22:28:41, dpapad wrote:
> This error should really be one of the two types. Is there a way to restore
this
> and keep the compiler happy?

I added a comment to track the expected type

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificates_browser_proxy.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificates_browser_proxy.js:18:
var Certificate;
On 2016/06/29 22:28:41, dpapad wrote:
> Some of the typedefs are moved to the new file and some stayed here. Is there
a
> rule about which should go in the new file?

It was only done as needed to compile. I've moved some of the 
types back into the proxy after changing the dependency between
the proxy and the types file.

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js:19:
this.$.dialog.open();
On 2016/06/29 22:28:41, dpapad wrote:
> I am surprised that this compiles. How does the compiler what is the type of
> this.$.dialog? Even if it does somehow, cr-dialog is not in the deps list.

IIUC, the $.foo get's translated to $['foo'] prior to the compile. That may
affect how the code is checked.

[dpapad, 2016-06-30 17:51:22]

LGTM with question.

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js:19:
this.$.dialog.open();

> IIUC, the $.foo get's translated to $['foo'] prior to the compile. That may
> affect how the code is checked.

cr-dialog has now a compile target, see
https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_dialog....
Can we cast this to CrDialogElement so that we have type coverage here. IIUC
currently the compiler skips type-checking because it does not know the type.

[dschuyler, 2016-06-30 20:52:15]

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
File
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js
(right):

https://codereview.chromium.org/2101243004/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/certificate_manager_page/certificates_error_dialog.js:19:
this.$.dialog.open();
On 2016/06/30 17:51:22, dpapad wrote:
> 
> > IIUC, the $.foo get's translated to $['foo'] prior to the compile. That may
> > affect how the code is checked.
> 
> cr-dialog has now a compile target, see
>
https://cs.chromium.org/chromium/src/ui/webui/resources/cr_elements/cr_dialog....
> Can we cast this to CrDialogElement so that we have type coverage here. IIUC
> currently the compiler skips type-checking because it does not know the type.

That's possible. Let's do that in a separate CL.

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

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2016-06-30 20:52:54]

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 21:25:16]

Description was changed from

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

to

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

[commit-bot: I haz the power, 2016-06-30 21:25:17]

Committed patchset #2 (id:60001)

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

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-30 21:26:43]

Description was changed from

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

to

==========
[MD settings] certificate manager closure compilation

This adds full closure compilation for the certificate manager page
directory.

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

Committed: https://crrev.com/d369e7530e080bad5679321998ea1f072b5f7d4d
Cr-Commit-Position: refs/heads/master@{#403293}
==========

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

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