Commit 73065253 authored by vitalyp's avatar vitalyp Committed by Commit bot

Add public API generation with cr.makePublic() and handle it in compiler pass

R=dbeam@chromium.org,tbreisacher@chromium.org
BUG=393873
TEST=third_party/closure_compiler/runner/how_to_test_compiler_pass.md

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

Cr-Commit-Position: refs/heads/master@{#295173}
parent 2bc03869
......@@ -444,18 +444,14 @@ cr.define('login', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(UserImageScreen, [
'setDefaultImages',
'setCameraPresent',
'setProfilePictureEnabled',
'setProfileImage',
'setSelectedImage',
'hideCurtain'
].forEach(function(name) {
UserImageScreen[name] = function(value) {
$('user-image')[name + '_'](value);
};
});
], $('user-image'));
return {
UserImageScreen: UserImageScreen
......
......@@ -12,6 +12,7 @@ cr.define('options', function() {
* AutomaticSettingsResetBanner class
* Provides encapsulated handling of the Reset Profile Settings banner.
* @constructor
* @extends {options.SettingsBannerBase}
*/
function AutomaticSettingsResetBanner() {}
......@@ -47,18 +48,36 @@ cr.define('options', function() {
PageManager.showPageByName('resetProfileSettings');
};
},
/**
* Called by the native code to show the banner if needed.
* @private
*/
show_: function() {
if (!this.hadBeenDismissed_) {
chrome.send('metricsHandler:recordAction', [this.showMetricName_]);
this.setVisibility(true);
}
},
/**
* Called when the banner should be closed as a result of something taking
* place on the WebUI page, i.e. when its close button is pressed, or when
* the confirmation dialog for the profile settings reset feature is opened.
* @private
*/
dismiss_: function() {
chrome.send(this.dismissNativeCallbackName_);
this.hadBeenDismissed_ = true;
this.setVisibility(false);
},
};
// Forward public APIs to private implementations.
[
cr.makePublic(AutomaticSettingsResetBanner, [
'show',
'dismiss',
].forEach(function(name) {
AutomaticSettingsResetBanner[name] = function() {
var instance = AutomaticSettingsResetBanner.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -17,10 +17,11 @@ cr.define('options', function() {
DEFAULT: 1
};
//
// BrowserOptions class
// Encapsulated handling of browser options page.
//
/**
* Encapsulated handling of browser options page.
* @constructor
* @extends {cr.ui.pageManager.Page}
*/
function BrowserOptions() {
Page.call(this, 'settings', loadTimeData.getString('settingsTitle'),
'settings');
......@@ -736,7 +737,7 @@ cr.define('options', function() {
section.classList.add('sliding');
// Force a style recalc before starting the animation.
/** @suppress {uselessCode} */
/** @suppress {suspiciousCode} */
section.offsetHeight;
section.style.height = (showing ? container.offsetHeight : 0) + 'px';
......@@ -1423,16 +1424,25 @@ cr.define('options', function() {
ManageProfileOverlay.showDeleteDialog(this.getCurrentProfile_());
},
/**
* @param {boolean} enabled
*/
setNativeThemeButtonEnabled_: function(enabled) {
var button = $('themes-native-button');
if (button)
button.disabled = !enabled;
},
/**
* @param {boolean} enabled
*/
setThemesResetButtonEnabled_: function(enabled) {
$('themes-reset').disabled = !enabled;
},
/**
* @param {boolean} managed
*/
setAccountPictureManaged_: function(managed) {
var picture = $('account-picture');
if (managed || UIAccountTweaks.loggedInAsGuest()) {
......@@ -1464,6 +1474,9 @@ cr.define('options', function() {
}
},
/**
* @param {boolean} managed
*/
setWallpaperManaged_: function(managed) {
var button = $('set-wallpaper');
button.disabled = !!managed;
......@@ -1915,7 +1928,7 @@ cr.define('options', function() {
};
//Forward public APIs to private implementations.
[
cr.makePublic(BrowserOptions, [
'addBluetoothDevice',
'deleteCurrentProfile',
'enableCertificateButton',
......@@ -1960,14 +1973,8 @@ cr.define('options', function() {
'updateEasyUnlock',
'updateManagesSupervisedUsers',
'updateSearchEngines',
'updateStartupPages',
'updateSyncState',
].forEach(function(name) {
BrowserOptions[name] = function() {
var instance = BrowserOptions.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
if (cr.isChromeOS) {
/**
......
......@@ -324,19 +324,14 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(ChangePictureOptions, [
'closeOverlay',
'setCameraPresent',
'setDefaultImages',
'setOldImage',
'setProfileImage',
'setSelectedImage',
].forEach(function(name) {
ChangePictureOptions[name] = function() {
var instance = ChangePictureOptions.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -161,15 +161,10 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(KeyboardOverlay, [
'showCapsLockOptions',
'showDiamondKeyOptions',
].forEach(function(name) {
KeyboardOverlay[name] = function() {
var instance = KeyboardOverlay.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -356,7 +356,7 @@ cr.define('options', function() {
* @param {boolean} nameIsUnique
*/
getImportHandler_: function(supervisedUser, nameIsUnique) {
return (function() {
return function() {
if (supervisedUser.needAvatar || !nameIsUnique) {
PageManager.showPageByName('supervisedUserImport');
} else {
......@@ -366,7 +366,7 @@ cr.define('options', function() {
[supervisedUser.name, supervisedUser.iconURL, false, true,
supervisedUser.id]);
}
}).bind(this);
}.bind(this);
},
/**
......@@ -597,7 +597,7 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(ManageProfileOverlay, [
'receiveDefaultProfileIconsAndNames',
'receiveNewProfileDefaults',
'receiveExistingProfileNames',
......@@ -608,12 +608,7 @@ cr.define('options', function() {
'showDeleteDialog',
'showDisconnectManagedProfileDialog',
'showCreateDialog',
].forEach(function(name) {
ManageProfileOverlay[name] = function() {
var instance = ManageProfileOverlay.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
function CreateProfileOverlay() {
Page.call(this, 'createProfile',
......@@ -853,7 +848,7 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(CreateProfileOverlay, [
'cancelCreateProfile',
'onError',
'onSuccess',
......@@ -861,12 +856,7 @@ cr.define('options', function() {
'updateCreateInProgress',
'updateSignedInStatus',
'updateSupervisedUsersAllowed',
].forEach(function(name) {
CreateProfileOverlay[name] = function() {
var instance = CreateProfileOverlay.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -232,16 +232,11 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations on the singleton instance.
[
cr.makePublic(PasswordManager, [
'setSavedPasswordsList',
'setPasswordExceptionsList',
'showPassword'
].forEach(function(name) {
PasswordManager[name] = function() {
var instance = PasswordManager.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -9,6 +9,7 @@ cr.define('options', function() {
/**
* Base class for banners that appear at the top of the settings page.
* @constructor
*/
function SettingsBannerBase() {}
......@@ -48,38 +49,14 @@ cr.define('options', function() {
*/
setVisibilibyDomElement_: null,
/**
* Called by the native code to show the banner if needed.
* @private
*/
show_: function() {
if (!this.hadBeenDismissed_) {
chrome.send('metricsHandler:recordAction', [this.showMetricName_]);
this.setVisibility_(true);
}
},
/**
* Called when the banner should be closed as a result of something taking
* place on the WebUI page, i.e. when its close button is pressed, or when
* the confirmation dialog for the profile settings reset feature is opened.
* @private
*/
dismiss_: function() {
chrome.send(this.dismissNativeCallbackName_);
this.hadBeenDismissed_ = true;
this.setVisibility_(false);
},
/**
* Sets whether or not the reset profile settings banner shall be visible.
* @param {boolean} show Whether or not to show the banner.
* @private
* @protected
*/
setVisibility_: function(show) {
setVisibility: function(show) {
this.setVisibilibyDomElement_.hidden = !show;
},
};
// Export
......
......@@ -155,15 +155,10 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(StartupOverlay, [
'updateStartupPages',
'updateAutocompleteSuggestions',
].forEach(function(name) {
StartupOverlay[name] = function() {
var instance = StartupOverlay.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -88,14 +88,9 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(SupervisedUserCreateConfirmOverlay, [
'setProfileInfo',
].forEach(function(name) {
SupervisedUserCreateConfirmOverlay[name] = function() {
var instance = SupervisedUserCreateConfirmOverlay.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -236,14 +236,9 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(SupervisedUserImportOverlay, [
'onSuccess',
].forEach(function(name) {
SupervisedUserImportOverlay[name] = function() {
var instance = SupervisedUserImportOverlay.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -134,19 +134,14 @@ cr.define('options', function() {
};
// Forward public APIs to private implementations.
[
cr.makePublic(SupervisedUserListData, [
'addObserver',
'onSigninError',
'receiveExistingSupervisedUsers',
'removeObserver',
'requestExistingSupervisedUsers',
'resetPromise',
].forEach(function(name) {
SupervisedUserListData[name] = function() {
var instance = SupervisedUserListData.getInstance();
return instance[name + '_'].apply(instance, arguments);
};
});
]);
// Export
return {
......
......@@ -19,7 +19,7 @@ Local modifications:
- Chrome-specific coding conventions to understand cr.addSingletonGetter().
- third_party/closure_compiler/runner/src/com/google/javascript/jscomp/ChromePass.java
Added pass to handle namespace definition with cr.define(), object chain
creation with cr.exportPath() and property definition with
{cr|Object}.defineProperty().
creation with cr.exportPath(), property definition with
{cr|Object}.defineProperty() and public API generation with cr.makePublic().
See third_party/closure_compiler/runner/how_to_test_compiler_pass.md for
testing instructions on this pass.
......@@ -37,6 +37,7 @@ public class ChromePass extends AbstractPostOrderCallback implements CompilerPas
private static final String CR_EXPORT_PATH = "cr.exportPath";
private static final String OBJECT_DEFINE_PROPERTY = "Object.defineProperty";
private static final String CR_DEFINE_PROPERTY = "cr.defineProperty";
private static final String CR_MAKE_PUBLIC = "cr.makePublic";
private static final String CR_DEFINE_COMMON_EXPLANATION = "It should be called like this:"
+ " cr.define('name.space', function() '{ ... return {Export: Internal}; }');";
......@@ -67,6 +68,19 @@ public class ChromePass extends AbstractPostOrderCallback implements CompilerPas
"Invalid cr.PropertyKind passed to cr.defineProperty(): expected ATTR,"
+ " BOOL_ATTR or JS, found \"{0}\".");
static final DiagnosticType CR_MAKE_PUBLIC_HAS_NO_JSDOC =
DiagnosticType.error("JSC_CR_MAKE_PUBLIC_HAS_NO_JSDOC",
"Private method exported by cr.makePublic() has no JSDoc.");
static final DiagnosticType CR_MAKE_PUBLIC_MISSED_DECLARATION =
DiagnosticType.error("JSC_CR_MAKE_PUBLIC_MISSED_DECLARATION",
"Method \"{1}_\" exported by cr.makePublic() on \"{0}\" has no declaration.");
static final DiagnosticType CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT =
DiagnosticType.error("JSC_CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT",
"Invalid second argument passed to cr.makePublic(): should be array of " +
"strings.");
public ChromePass(AbstractCompiler compiler) {
this.compiler = compiler;
// The global variable "cr" is declared in ui/webui/resources/js/cr.js.
......@@ -92,6 +106,10 @@ public class ChromePass extends AbstractPostOrderCallback implements CompilerPas
callee.matchesQualifiedName(CR_DEFINE_PROPERTY)) {
visitPropertyDefinition(node, parent);
compiler.reportCodeChange();
} else if (callee.matchesQualifiedName(CR_MAKE_PUBLIC)) {
if (visitMakePublic(node, parent)) {
compiler.reportCodeChange();
}
}
}
}
......@@ -140,6 +158,98 @@ public class ChromePass extends AbstractPostOrderCallback implements CompilerPas
target.setJSDocInfo(builder.build(target));
}
private boolean visitMakePublic(Node call, Node exprResult) {
boolean changesMade = false;
Node scope = exprResult.getParent();
String className = call.getChildAtIndex(1).getQualifiedName();
String prototype = className + ".prototype";
Node methods = call.getChildAtIndex(2);
if (methods == null || !methods.isArrayLit()) {
compiler.report(JSError.make(exprResult, CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT));
return changesMade;
}
Set<String> methodNames = new HashSet<>();
for (Node methodName: methods.children()) {
if (!methodName.isString()) {
compiler.report(JSError.make(methodName, CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT));
return changesMade;
}
methodNames.add(methodName.getString());
}
for (Node child: scope.children()) {
if (isAssignmentToPrototype(child, prototype)) {
Node objectLit = child.getFirstChild().getChildAtIndex(1);
for (Node stringKey : objectLit.children()) {
String field = stringKey.getString();
changesMade |= maybeAddPublicDeclaration(field, methodNames, className,
stringKey, scope, exprResult);
}
} else if (isAssignmentToPrototypeMethod(child, prototype)) {
Node assignNode = child.getFirstChild();
String qualifiedName = assignNode.getFirstChild().getQualifiedName();
String field = qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1);
changesMade |= maybeAddPublicDeclaration(field, methodNames, className,
assignNode, scope, exprResult);
} else if (isDummyPrototypeMethodDeclaration(child, prototype)) {
String qualifiedName = child.getFirstChild().getQualifiedName();
String field = qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1);
changesMade |= maybeAddPublicDeclaration(field, methodNames, className,
child.getFirstChild(), scope, exprResult);
}
}
for (String missedDeclaration : methodNames) {
compiler.report(JSError.make(exprResult, CR_MAKE_PUBLIC_MISSED_DECLARATION, className,
missedDeclaration));
}
return changesMade;
}
private boolean isAssignmentToPrototype(Node node, String prototype) {
Node assignNode;
return node.isExprResult() && (assignNode = node.getFirstChild()).isAssign() &&
assignNode.getFirstChild().getQualifiedName().equals(prototype);
}
private boolean isAssignmentToPrototypeMethod(Node node, String prototype) {
Node assignNode;
return node.isExprResult() && (assignNode = node.getFirstChild()).isAssign() &&
assignNode.getFirstChild().getQualifiedName().startsWith(prototype + ".");
}
private boolean isDummyPrototypeMethodDeclaration(Node node, String prototype) {
Node getPropNode;
return node.isExprResult() && (getPropNode = node.getFirstChild()).isGetProp() &&
getPropNode.getQualifiedName().startsWith(prototype + ".");
}
private boolean maybeAddPublicDeclaration(String field, Set<String> publicAPIStrings,
String className, Node jsDocSourceNode, Node scope, Node exprResult) {
boolean changesMade = false;
if (field.endsWith("_")) {
String publicName = field.substring(0, field.length() - 1);
if (publicAPIStrings.contains(publicName)) {
Node methodDeclaration = NodeUtil.newQualifiedNameNode(
compiler.getCodingConvention(), className + "." + publicName);
if (jsDocSourceNode.getJSDocInfo() != null) {
methodDeclaration.setJSDocInfo(jsDocSourceNode.getJSDocInfo());
scope.addChildBefore(
IR.exprResult(methodDeclaration).srcrefTree(exprResult),
exprResult);
changesMade = true;
} else {
compiler.report(JSError.make(jsDocSourceNode, CR_MAKE_PUBLIC_HAS_NO_JSDOC));
}
publicAPIStrings.remove(publicName);
}
}
return changesMade;
}
private void visitExportPath(Node crExportPathNode, Node parent) {
if (crExportPathNode.getChildCount() != 2) {
compiler.report(JSError.make(crExportPathNode,
......@@ -342,5 +452,4 @@ public class ChromePass extends AbstractPostOrderCallback implements CompilerPas
this.namespaceName + "." + externalName).srcrefTree(internalName);
}
}
}
......@@ -363,4 +363,166 @@ public class ChromePassTest extends CompilerTestCase {
test("cr.exportPath();", null, ChromePass.CR_EXPORT_PATH_WRONG_NUMBER_OF_ARGUMENTS);
}
public void testCrMakePublicWorksOnOneMethodDefinedInPrototypeObject() throws Exception {
test(
"/** @constructor */\n" +
"function Class() {};\n" +
"\n" +
"Class.prototype = {\n" +
" /** @return {number} */\n" +
" method_: function() { return 42; }\n" +
"};\n" +
"\n" +
"cr.makePublic(Class, ['method']);",
"/** @constructor */\n" +
"function Class() {};\n" +
"\n" +
"Class.prototype = {\n" +
" /** @return {number} */\n" +
" method_: function() { return 42; }\n" +
"};\n" +
"\n" +
"/** @return {number} */\n" +
"Class.method;\n" +
"\n" +
"cr.makePublic(Class, ['method']);");
}
public void testCrMakePublicWorksOnTwoMethods() throws Exception {
test(
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"Class.prototype = {\n" +
" /** @return {number} */\n" +
" m1_: function() { return 42; },\n" +
"\n" +
" /** @return {string} */\n" +
" m2_: function() { return ''; }\n" +
"};\n" +
"\n" +
"cr.makePublic(Class, ['m1', 'm2']);",
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"Class.prototype = {\n" +
" /** @return {number} */\n" +
" m1_: function() { return 42; },\n" +
"\n" +
" /** @return {string} */\n" +
" m2_: function() { return ''; }\n" +
"}\n" +
"\n" +
"/** @return {number} */\n" +
"Class.m1;\n" +
"\n" +
"/** @return {string} */\n" +
"Class.m2;\n" +
"\n" +
"cr.makePublic(Class, ['m1', 'm2']);");
}
public void testCrMakePublicRequiresMethodsToHaveJSDoc() throws Exception {
test("/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"Class.prototype = {\n" +
" method_: function() {}\n" +
"}\n" +
"\n" +
"cr.makePublic(Class, ['method']);", null, ChromePass.CR_MAKE_PUBLIC_HAS_NO_JSDOC);
}
public void testCrMakePublicDoesNothingWithMethodsNotInAPI() throws Exception {
test(
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"Class.prototype = {\n" +
" method_: function() {}\n" +
"}\n" +
"\n" +
"cr.makePublic(Class, []);",
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"Class.prototype = {\n" +
" method_: function() {}\n" +
"}\n" +
"\n" +
"cr.makePublic(Class, []);");
}
public void testCrMakePublicRequiresExportedMethodToBeDeclared() throws Exception {
test(
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"Class.prototype = {\n" +
"}\n" +
"\n" +
"cr.makePublic(Class, ['method']);", null,
ChromePass.CR_MAKE_PUBLIC_MISSED_DECLARATION);
}
public void testCrMakePublicWorksOnOneMethodDefinedDirectlyOnPrototype() throws Exception {
test(
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"/** @return {number} */\n" +
"Class.prototype.method_ = function() {};\n" +
"\n" +
"cr.makePublic(Class, ['method']);",
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"/** @return {number} */\n" +
"Class.prototype.method_ = function() {};\n" +
"\n" +
"/** @return {number} */\n" +
"Class.method;\n" +
"\n" +
"cr.makePublic(Class, ['method']);");
}
public void testCrMakePublicWorksOnDummyDeclaration() throws Exception {
test(
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"/** @return {number} */\n" +
"Class.prototype.method_;\n" +
"\n" +
"cr.makePublic(Class, ['method']);",
"/** @constructor */\n" +
"function Class() {}\n" +
"\n" +
"/** @return {number} */\n" +
"Class.prototype.method_;\n" +
"\n" +
"/** @return {number} */\n" +
"Class.method;\n" +
"\n" +
"cr.makePublic(Class, ['method']);");
}
public void testCrMakePublicReportsInvalidSecondArgumentMissing() throws Exception {
test(
"cr.makePublic(Class);", null,
ChromePass.CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT);
}
public void testCrMakePublicReportsInvalidSecondArgumentNotAnArray() throws Exception {
test(
"cr.makePublic(Class, 42);", null,
ChromePass.CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT);
}
public void testCrMakePublicReportsInvalidSecondArgumentArrayWithNotAString() throws Exception {
test(
"cr.makePublic(Class, [42]);", null,
ChromePass.CR_MAKE_PUBLIC_INVALID_SECOND_ARGUMENT);
}
}
......@@ -294,6 +294,23 @@ var cr = function() {
};
}
/**
* Forwards public APIs to private implementations.
* @param {Function} ctor Constructor that have private implementations in its
* prototype.
* @param {Array.<string>} methods List of public method names that have their
* underscored counterparts in constructor's prototype.
* @param {*=} opt_target Target node.
*/
function makePublic(ctor, methods, opt_target) {
methods.forEach(function(method) {
ctor[method] = function() {
var target = opt_target || ctor.getInstance();
return target[method + '_'].apply(target, arguments);
};
});
}
return {
addSingletonGetter: addSingletonGetter,
createUid: createUid,
......@@ -303,6 +320,7 @@ var cr = function() {
dispatchSimpleEvent: dispatchSimpleEvent,
exportPath: exportPath,
getUid: getUid,
makePublic: makePublic,
PropertyKind: PropertyKind,
get doc() {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment