Skip to content

Commit

Permalink
Update Chromoting to use /third_party/closure_compiler.
Browse files Browse the repository at this point in the history
This cl updates the checker.py script in /third_party/closure_compiler to
add an option to process a set of JS files at once rather than one at a
time (the current default). It also adds a flag to enable additional compiler
checks (since Chromoting wants all checks enabled).

It also updates the Chromoting .gyp files to make use of the new compiler.

The remaining changes were motivated by the errors reported by the new
closure compiler.

BUG=

Review URL: https://codereview.chromium.org/803653004

Cr-Commit-Position: refs/heads/master@{#311531}
  • Loading branch information
garykac authored and Commit bot committed Jan 14, 2015
1 parent 5d49353 commit 23605fe
Show file tree
Hide file tree
Showing 62 changed files with 541 additions and 647 deletions.
16 changes: 8 additions & 8 deletions remoting/app_remoting_webapp_build.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@
'success_stamp': '<(PRODUCT_DIR)/>(_target_name)_main_jscompile.stamp',
},
'inputs': [
'tools/jscompile.py',
'<@(ar_main_js_files)',
'<@(remoting_webapp_js_proto_files)',
# Include zip as input so that this action is run after the build.
Expand All @@ -268,11 +267,12 @@
'<(success_stamp)',
],
'action': [
'python', 'tools/jscompile.py',
'python', '../third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp)',
'<@(ar_main_js_files)',
'<@(remoting_webapp_js_proto_files)',
'--success-stamp',
'<(success_stamp)'
],
},
{
Expand All @@ -281,7 +281,6 @@
'success_stamp': '<(PRODUCT_DIR)/>(_target_name)_background_jscompile.stamp',
},
'inputs': [
'tools/jscompile.py',
'<@(ar_background_js_files)',
'<@(remoting_webapp_js_proto_files)',
# Include zip as input so that this action is run after the build.
Expand All @@ -291,11 +290,12 @@
'<(success_stamp)',
],
'action': [
'python', 'tools/jscompile.py',
'python', '../third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp)',
'<@(ar_background_js_files)',
'<@(remoting_webapp_js_proto_files)',
'--success-stamp',
'<(success_stamp)'
],
},
], # actions
Expand Down
6 changes: 4 additions & 2 deletions remoting/remoting_webapp.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@
'<(success_stamp)',
],
'action': [
'python', 'tools/jscompile.py',
'python', '../third_party/closure_compiler/checker.py',
'--strict',
'--no-single-file',
'--success-stamp', '<(success_stamp)',
'<@(remoting_webapp_crd_js_files)',
'<@(remoting_webapp_js_proto_files)',
'--success-stamp', '<(success_stamp)'
],
},
], # actions
Expand Down
3 changes: 1 addition & 2 deletions remoting/remoting_webapp_files.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# These provide type information for jscompile.
'remoting_webapp_js_proto_files': [
'webapp/js_proto/chrome_proto.js',
'webapp/js_proto/console_proto.js',
'webapp/js_proto/dom_proto.js',
'webapp/js_proto/remoting_proto.js',
],
Expand Down Expand Up @@ -167,8 +166,8 @@
],
# The unit test cases for the webapp
'remoting_webapp_unittest_js_files': [
'webapp/js_proto/chrome_proto.js',
'webapp/unittests/chrome_mocks.js',
'webapp/js_proto/chrome_proto.js',
'webapp/unittests/base_unittest.js',
'webapp/unittests/it2me_helpee_channel_unittest.js',
'webapp/unittests/it2me_helper_channel_unittest.js',
Expand Down
2 changes: 1 addition & 1 deletion remoting/webapp/app_remoting/js/app_remoting.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ remoting.AppRemoting.prototype.handleConnected = function(clientSession) {
// Cancel the ping when the connection closes.
clientSession.addEventListener(
remoting.ClientSession.Events.stateChanged,
/** @param {remoting.ClientSession.StateEvent} state */
/** @param {remoting.ClientSession.StateEvent=} state */
function(state) {
if (state.current === remoting.ClientSession.State.CLOSED ||
state.current === remoting.ClientSession.State.FAILED) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ remoting.ApplicationContextMenu.prototype.updateConnectionRTT =
rttText));
};

/** @param {OnClickData} info */
/** @param {OnClickData=} info */
remoting.ApplicationContextMenu.prototype.onClicked_ = function(info) {
switch (info.menuItemId) {

Expand Down
2 changes: 1 addition & 1 deletion remoting/webapp/app_remoting/js/context_menu_adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ remoting.ContextMenuAdapter.prototype.remove = function(id) {
};

/**
* @param {function(OnClickData):void} listener
* @param {function(OnClickData=):void} listener
*/
remoting.ContextMenuAdapter.prototype.addListener = function(listener) {
};
12 changes: 6 additions & 6 deletions remoting/webapp/app_remoting/js/context_menu_dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,24 @@ remoting.ContextMenuDom = function(root) {
* @private
*/
this.stub_ = /** @type {HTMLElement} */
this.root_.querySelector('.context-menu-stub');
(this.root_.querySelector('.context-menu-stub'));
/**
* @type {HTMLElement}
* @private
*/
this.icon_ = /** @type {HTMLElement} */
this.root_.querySelector('.context-menu-icon');
(this.root_.querySelector('.context-menu-icon'));
/**
* @type {HTMLElement}
* @private
*/
this.screen_ = /** @type {HTMLElement} */
this.root_.querySelector('.context-menu-screen');
(this.root_.querySelector('.context-menu-screen'));
/**
* @type {HTMLElement}
* @private
*/
this.menu_ = /** @type {HTMLElement} */ this.root_.querySelector('ul');
this.menu_ = /** @type {HTMLElement} */ (this.root_.querySelector('ul'));
/**
* @type {number}
* @private
Expand All @@ -84,7 +84,7 @@ remoting.ContextMenuDom = function(root) {
*/
this.stubDragged_ = false;

/*
/**
* @private
*/
this.dragAndDrop_ = new remoting.DragAndDrop(
Expand Down Expand Up @@ -194,7 +194,7 @@ remoting.ContextMenuDom.prototype.remove = function(id) {
};

/**
* @param {function(OnClickData):void} listener
* @param {function(OnClickData=):void} listener
*/
remoting.ContextMenuDom.prototype.addListener = function(listener) {
this.eventSource_.addEventListener(this.eventName_, listener);
Expand Down
6 changes: 3 additions & 3 deletions remoting/webapp/app_remoting/js/drag_and_drop.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,13 @@ remoting.DragAndDrop = function(element, dragUpdate,
this.seenNonZeroDelta_ = false;

/**
* @type {function():void}
* @type {function(Event):void}
* @private
*/
this.callOnMouseUp_ = this.onMouseUp_.bind(this);

/**
* @type {function():void}
* @type {function(Event):void}
* @private
*/
this.callOnMouseMove_ = this.onMouseMove_.bind(this);
Expand Down Expand Up @@ -139,4 +139,4 @@ remoting.DragAndDrop.prototype.onMouseUp_ = function(event) {
if (this.dragEnd_) {
this.dragEnd_();
}
};
};
2 changes: 1 addition & 1 deletion remoting/webapp/app_remoting/js/keyboard_layouts_menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ remoting.KeyboardLayoutsMenu.prototype.makeMenuId_ = function(layout) {
/**
* Handle a click on the application's context menu.
*
* @param {OnClickData} info
* @param {OnClickData=} info
* @private
*/
remoting.KeyboardLayoutsMenu.prototype.onContextMenu_ = function(info) {
Expand Down
2 changes: 1 addition & 1 deletion remoting/webapp/app_remoting/js/window_activation_menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ remoting.WindowActivationMenu.prototype.makeMenuId_ = function(windowId) {
/**
* Handle a click on the application's context menu.
*
* @param {OnClickData} info
* @param {OnClickData=} info
* @private
*/
remoting.WindowActivationMenu.prototype.onContextMenu_ = function(info) {
Expand Down
2 changes: 1 addition & 1 deletion remoting/webapp/base/js/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ remoting.Application.prototype.onVideoStreamingStarted = function() {
* @return {boolean} Return true if the extension message was recognized.
*/
remoting.Application.prototype.onExtensionMessage = function(type, data) {
var message = /** @type {Object} */base.jsonParseSafe(data);
var message = /** @type {Object} */ (base.jsonParseSafe(data));
if (typeof message != 'object') {
return false;
}
Expand Down
38 changes: 19 additions & 19 deletions remoting/webapp/base/js/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ base.debug.assert = function(expr, opt_msg) {
base.debug.callstack = function() {
try {
throw new Error();
} catch (e) {
var error = /** @type {Error} */ e;
} catch (/** @type {Error} */ error) {
var callstack = error.stack
.replace(/^\s+(at eval )?at\s+/gm, '') // Remove 'at' and indentation.
.split('\n');
Expand Down Expand Up @@ -91,7 +90,7 @@ base.mix = function(dest, src) {
* Adds a mixin to a class.
* @param {Object} dest
* @param {Object} src
* @suppress {checkTypes}
* @suppress {checkTypes|reportUnknownTypes}
*/
base.extend = function(dest, src) {
base.mix(dest.prototype, src.prototype || src);
Expand Down Expand Up @@ -195,32 +194,32 @@ base.escapeHTML = function(str) {
*/
base.Deferred = function() {
/**
* @type {?function(?=)}
* @type {?function(?):void}
* @private
*/
this.resolve_ = null;

/**
* @type {?function(?)}
* @type {?function(?):void}
* @private
*/
this.reject_ = null;

/**
* @this {base.Deferred}
* @param {function(?):void} resolve
* @param {function(*):void} reject
*/
var initPromise = function(resolve, reject) {
this.resolve_ = resolve;
this.reject_ = reject;
};

/**
* @type {Promise}
* @private
*/
this.promise_ = new Promise(
/**
* @param {function(?=):void} resolve
* @param {function(?):void} reject
* @this {base.Deferred}
*/
function(resolve, reject) {
this.resolve_ = resolve;
this.reject_ = reject;
}.bind(this)
);
this.promise_ = new Promise(initPromise.bind(this));
};

/** @param {*} reason */
Expand All @@ -246,7 +245,7 @@ base.Promise = function() {};
*/
base.Promise.sleep = function(delay) {
return new Promise(
/** @param {function():void} fulfill */
/** @param {function(*):void} fulfill */
function(fulfill) {
window.setTimeout(fulfill, delay);
});
Expand Down Expand Up @@ -436,11 +435,12 @@ base.generateXsrfToken = function() {

/**
* @param {string} jsonString A JSON-encoded string.
* @return {*} The decoded object, or undefined if the string cannot be parsed.
* @return {Object|undefined} The decoded object, or undefined if the string
* cannot be parsed.
*/
base.jsonParseSafe = function(jsonString) {
try {
return JSON.parse(jsonString);
return /** @type {Object} */ (JSON.parse(jsonString));
} catch (err) {
return undefined;
}
Expand Down
8 changes: 5 additions & 3 deletions remoting/webapp/base/js/message_window.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,17 @@ MessageWindowImpl.prototype.onMessage_ = function(event) {
infoboxDiv.hidden = true;
}

var messageIdStr = messageId.toString();

this.initButton_(
button,
buttonLabel,
this.sendReply_.bind(this, event.source, messageId, 1));
this.sendReply_.bind(this, event.source, messageIdStr, 1));

this.initButton_(
cancelButton,
cancelButtonLabel,
this.sendReply_.bind(this, event.source, messageId, 0));
this.sendReply_.bind(this, event.source, messageIdStr, 0));

var buttonToFocus = (cancelButtonLabel) ? cancelButton : button;
buttonToFocus.focus();
Expand All @@ -143,7 +145,7 @@ MessageWindowImpl.prototype.onMessage_ = function(event) {
// Note that when a button is pressed, this will result in sendReply_
// being called multiple times (once for the button, once for close).
chrome.app.window.current().onClosed.addListener(
this.sendReply_.bind(this, event.source, messageId, 0));
this.sendReply_.bind(this, event.source, messageIdStr, 0));

this.updateSize_();
chrome.app.window.current().show();
Expand Down
Loading

0 comments on commit 23605fe

Please sign in to comment.