WebUI TaskManager: Delay scripts loading.

WebUI TaskManager: Delay scripts loading.

This CL delays loading of some scripts. Previous commit (http://crrev/113772) was reverted, because it increased the resource size of chromium. This CL picked up the delay loading part from it.

This CL also contains calculation of the loading and initialization times.

BUG=104136
TEST=manual on linux

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=118303

[yoshiki, 2011-12-19 15:52:46]

@mazda for review and @jhawkins for approval. Please take a look.

[James Hawkins, 2011-12-19 18:35:55]

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/includes.js:52: var loadDelayedIncludes;
Why is this here?

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:232: * @public
In general we don't specify @public since it's implied.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/metrics.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:47: if (name in
metrics.intervals) {
Remove braces.

[arv (Not doing code reviews), 2011-12-19 22:57:14]

I wonder if you could change to deferred or async scripts instead?

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:167: if (!eval(expr))
eval should be prevented if possible. What is its purpose here?

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:408: var addContextMenu =
function() {
This will not work correctly. label changes as you iterate and this just
captures the last one.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/metrics.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:11: var metrics = {};
I'm a bit confused. I thought this would do a chrome.send so we could get
histograms for this. Now I see that all it does is to print stuff to console.

console has API for timing things already and you could use that to reduce the
code here.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:28: var args =
Array.apply(null, arguments);
This is error prone. If argument.length == 1 and arguments[0] is a number it
will use that as the length.

A better way is to use slice

var args = Array.prototype.slice.call(arguments);

but it is unclear why you want to convert the argument object to a normal array
here?

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:31: // TODO(yoshiki): Enables
the recoading metrixes of loading times.
Is "metrixes" a real word?

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:32: //
chrome.metricsPrivate[name].apply(chrome.metricsPrivate, args);
Look into chrome.send. We already have a chrome send for UMA stats.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:35: if
(localStorage.logMetrics) {
no {} here

[yoshiki, 2011-12-20 15:18:24]

PTAL

On 2011/12/19 22:57:14, arv wrote:
> I wonder if you could change to deferred or async scripts instead?

The scrips have dependency so that we can't use async.

deferred may be able to use. But loading of delayed scripts may interrupt before
initialization of taskmanager. We want to use task manager as soon as possible,
so that delayed scripts are loaded after the initialization.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/includes.js:52: var loadDelayedIncludes;
> Why is this here?

This variable will be set in the function below, to bind 2 variables (isHarness
and prefix).

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:167: if (!eval(expr))
On 2011/12/19 22:57:14, arv wrote:
> eval should be prevented if possible. What is its purpose here?

Done.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:232: * @public
On 2011/12/19 18:35:56, James Hawkins wrote:
> In general we don't specify @public since it's implied.

Done.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:408: var addContextMenu =
function() {
On 2011/12/19 22:57:14, arv wrote:
> This will not work correctly. label changes as you iterate and this just
> captures the last one.

Done.

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
File chrome/browser/resources/task_manager/metrics.js (right):

http://codereview.chromium.org/8993007/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/metrics.js:11: var metrics = {};
You're right. I intended to add recording histograms in future. But it has not
been implemented yet, so that I changed the file name and use console.time API
instead of console.log.

On 2011/12/19 22:57:14, arv wrote:
> I'm a bit confused. I thought this would do a chrome.send so we could get
> histograms for this. Now I see that all it does is to print stuff to console.
> 
> console has API for timing things already and you could use that to reduce the
> code here.

[arv (Not doing code reviews), 2011-12-20 19:33:56]

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:246: var column_id =
DEFAULT_COLUMNS[j][0];
no underscores in js

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:249: var process_id =
dm.item(i)['processId'][0];
no underscores in js

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:250: var label_id = 'detail-' +
column_id + '-pid' + process_id + '-' + k;
no underscores in js

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:416: label.setContextMenu =
function(taskmanager) {
This looks strange. The intended usage was:

cr.ui.contextMenuHandler.addContextMenuProperty(label);
...
label.contextMenu = menuElement;

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/measuretime.js (right):

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:14: if
(localStorage.isShowTime)
Does this change? Accessing stuff on localStorage is pretty slow.

[yoshiki, 2011-12-21 05:18:56]

Thanks. PTAL?

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:246: var column_id =
DEFAULT_COLUMNS[j][0];
On 2011/12/20 19:33:57, arv wrote:
> no underscores in js

Done.

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:249: var process_id =
dm.item(i)['processId'][0];
On 2011/12/20 19:33:57, arv wrote:
> no underscores in js

Done.

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:250: var label_id = 'detail-' +
column_id + '-pid' + process_id + '-' + k;
On 2011/12/20 19:33:57, arv wrote:
> no underscores in js

Done.

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:416: label.setContextMenu =
function(taskmanager) {
On 2011/12/20 19:33:57, arv wrote:
> This looks strange. The intended usage was:
> 
> cr.ui.contextMenuHandler.addContextMenuProperty(label);
> ...
> label.contextMenu = menuElement;

Done.

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/measuretime.js (right):

http://codereview.chromium.org/8993007/diff/12004/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:14: if
(localStorage.isShowTime)
On 2011/12/20 19:33:57, arv wrote:
> Does this change? Accessing stuff on localStorage is pretty slow.

Done. The value is stored at the time of initialization.

[mazda, 2011-12-21 05:56:52]

lgtm with nits.

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:75: loadDelayedIncludes =
function (taskmanager) {
No space between 'function' and '('.

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:239: delayedInitialize: function
() {
No space between 'function' and '('.

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/measuretime.js (right):

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:13: measuretime.isShowTime
= localStorage.isShowTime;
localStorage.measureTimeEnabled may be better?

[yoshiki, 2011-12-21 06:08:47]

mazda: Thanks!

jhawkins, arv: PTAL?

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:75: loadDelayedIncludes =
function (taskmanager) {
On 2011/12/21 05:56:53, mazda wrote:
> No space between 'function' and '('.

Done.

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:239: delayedInitialize: function
() {
On 2011/12/21 05:56:53, mazda wrote:
> No space between 'function' and '('.

Done.

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/measuretime.js (right):

http://codereview.chromium.org/8993007/diff/15001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:13: measuretime.isShowTime
= localStorage.isShowTime;
On 2011/12/21 05:56:53, mazda wrote:
> localStorage.measureTimeEnabled may be better?

Done.

[James Hawkins, 2011-12-21 17:31:49]

SLGTM

[arv (Not doing code reviews), 2011-12-21 20:19:42]

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:67: document.write('<script
src="' + prefix + 'js/' + script[i] +
Using document.write slows down loading. It prevents look ahead parsing and
fetching. Please consider moving these back to the html file.

Same goes for the css.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:86: s.type =
'text/javascript';
useless statement

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:218: // <if
expr="pp_ifdef('chromeos')">
Can this be removed? The pre-processor does all of this for you.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:416: label.setContextMenu =
function(contextMenuElement) {
I still think this is wrong.

Can you at least add a comment explaining why the straight forward way does not
work?

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/measuretime.js (right):

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:11: var measuretime = {};
Why not measureTime?

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:11: var measuretime = {};
Using an object literal is more concise


var measuretime = {
  isEnabled: localStorage.measureTimeEnabled,
  startInterval: function(name) {
    ...
  {,
  recordInterval: ...
};

[yoshiki, 2011-12-22 08:50:30]

Thanks. PTAL

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:67: document.write('<script
src="' + prefix + 'js/' + script[i] +
Thank you for advice, I'll consider it. I have a plan to change the way as same
as file manager (<script> tags on html, and enabled to use mock).

On 2011/12/21 20:19:43, arv wrote:
> Using document.write slows down loading. It prevents look ahead parsing and
> fetching. Please consider moving these back to the html file.
> 
> Same goes for the css.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:86: s.type =
'text/javascript';
On 2011/12/21 20:19:43, arv wrote:
> useless statement

Done.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:218: // <if
expr="pp_ifdef('chromeos')">
In case of development using mock, these comment slashes are needed.

On 2011/12/21 20:19:43, arv wrote:
> Can this be removed? The pre-processor does all of this for you.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:416: label.setContextMenu =
function(contextMenuElement) {
On 2011/12/21 20:19:43, arv wrote:
> I still think this is wrong.
> 
> Can you at least add a comment explaining why the straight forward way does
not
> work?

Done. I added a comment.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/measuretime.js (right):

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:11: var measuretime = {};
On 2011/12/21 20:19:43, arv wrote:
> Using an object literal is more concise
> 
> 
> var measuretime = {
>   isEnabled: localStorage.measureTimeEnabled,
>   startInterval: function(name) {
>     ...
>   {,
>   recordInterval: ...
> };

Done.

http://codereview.chromium.org/8993007/diff/17003/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/measuretime.js:11: var measuretime = {};
On 2011/12/21 20:19:43, arv wrote:
> Why not measureTime?

Done.

[arv (Not doing code reviews), 2011-12-22 20:49:13]

http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:416: // If the delayed include
files (included in includes.js) ware not
typo

http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:420: label.setContextMenu =
function(contextMenuElement) {
OK. I almost get it now. Can this be done as a function instead of a method?

function setContextMenu(element, menuElement) {
  ...
}

I find the line

if (label && label.setContextMenu)

to be very confusing. When would you get a label that was not created by
renderColumn_?

[yoshiki, 2011-12-26 04:40:58]

PTAL

http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:416: // If the delayed include
files (included in includes.js) ware not
On 2011/12/22 20:49:14, arv wrote:
> typo

Done.

http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:420: label.setContextMenu =
function(contextMenuElement) {
> to be very confusing. When would you get a label that was not created by
> renderColumn_?

This is when all the delayed scripts have not loaded. If renderColumn_ is called
before loading of delayed scripts, context menu is loaded at delayedInitialize()
instead of renderColumn_().

On 2011/12/22 20:49:14, arv wrote:
> OK. I almost get it now. Can this be done as a function instead of a method?
> 
> function setContextMenu(element, menuElement) {
>   ...
> }
> 
> I find the line
> 
> if (label && label.setContextMenu)
> 
> to be very confusing. When would you get a label that was not created by
> renderColumn_?

[yoshiki, 2012-01-11 14:17:21]

arv: ping?

On 2011/12/26 04:40:58, yoshiki wrote:
> PTAL
> 
>
http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
> File chrome/browser/resources/task_manager/main.js (right):
> 
>
http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
> chrome/browser/resources/task_manager/main.js:416: // If the delayed include
> files (included in includes.js) ware not
> On 2011/12/22 20:49:14, arv wrote:
> > typo
> 
> Done.
> 
>
http://codereview.chromium.org/8993007/diff/22001/chrome/browser/resources/ta...
> chrome/browser/resources/task_manager/main.js:420: label.setContextMenu =
> function(contextMenuElement) {
> > to be very confusing. When would you get a label that was not created by
> > renderColumn_?
> 
> This is when all the delayed scripts have not loaded. If renderColumn_ is
called
> before loading of delayed scripts, context menu is loaded at
delayedInitialize()
> instead of renderColumn_().
> 
> On 2011/12/22 20:49:14, arv wrote:
> > OK. I almost get it now. Can this be done as a function instead of a method?
> > 
> > function setContextMenu(element, menuElement) {
> >   ...
> > }
> > 
> > I find the line
> > 
> > if (label && label.setContextMenu)
> > 
> > to be very confusing. When would you get a label that was not created by
> > renderColumn_?

[James Hawkins, 2012-01-11 16:16:25]

arv is OOO till 1/15.

[yoshiki, 2012-01-12 11:31:37]

ok, thanks.

On 2012/01/11 16:16:25, James Hawkins wrote:
> arv is OOO till 1/15.

[arv (Not doing code reviews), 2012-01-17 21:47:06]

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/includes.js (right):

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:80: for (var i = 0; i <
scriptDelayed.length; ++i) {
Prefer i++ in JS

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/includes.js:87:
document.body.appendChild(s);
document.head.appendChild seems slightly more correct

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
File chrome/browser/resources/task_manager/main.js (right):

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:215: * This method is called by
initialize().
@private

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:396: setContextMenu_:
function(element, contextMenuElement) {
I've said this before but this is too hacky. How about adding a way to set the
context menu of any element in cr.ui.contextMenuHandler instead?

http://codereview.chromium.org/8993007/diff/26001/chrome/browser/resources/ta...
chrome/browser/resources/task_manager/main.js:428: this.setContextMenu_(label,
this.tableContextMenu_);
... something like:

cr.ui.contextMenuHandler.setContextMenu(label, menu)

[yoshiki, 2012-01-18 16:54:28]

arv: Thank you for comments. PTAL

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
File chrome/browser/resources/task_manager/includes.js (right):

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
chrome/browser/resources/task_manager/includes.js:80: for (var i = 0; i <
scriptDelayed.length; ++i) {
On 2012/01/17 21:47:06, arv wrote:
> Prefer i++ in JS

Done.

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
chrome/browser/resources/task_manager/includes.js:87:
document.body.appendChild(s);
On 2012/01/17 21:47:06, arv wrote:
> document.head.appendChild seems slightly more correct

Done.

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
File chrome/browser/resources/task_manager/main.js (right):

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
chrome/browser/resources/task_manager/main.js:215: * This method is called by
initialize().
On 2012/01/17 21:47:06, arv wrote:
> @private

Done.

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
chrome/browser/resources/task_manager/main.js:396: setContextMenu_:
function(element, contextMenuElement) {
Thanks. Added cr.ui.contextMenuHandler.setContextMenu() instead of this.

On 2012/01/17 21:47:06, arv wrote:
> I've said this before but this is too hacky. How about adding a way to set the
> context menu of any element in cr.ui.contextMenuHandler instead?

https://chromiumcodereview.appspot.com/8993007/diff/26001/chrome/browser/reso...
chrome/browser/resources/task_manager/main.js:428: this.setContextMenu_(label,
this.tableContextMenu_);
On 2012/01/17 21:47:06, arv wrote:
> ... something like:
> 
> cr.ui.contextMenuHandler.setContextMenu(label, menu)

Done.

[arv (Not doing code reviews), 2012-01-18 19:19:49]

lgtm

https://chromiumcodereview.appspot.com/8993007/diff/36006/chrome/browser/reso...
File chrome/browser/resources/shared/js/cr/ui/context_menu_handler.js (right):

https://chromiumcodereview.appspot.com/8993007/diff/36006/chrome/browser/reso...
chrome/browser/resources/shared/js/cr/ui/context_menu_handler.js:219: * @param
{!Element|!Function} element The element or class to set the
This should probably only be {!Element} even though Function would work but
calling this on a function seems wrong from an API perspective.

[commit-bot: I haz the power, 2012-01-19 15:17:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yoshiki@chromium.org/8993007/42002

[commit-bot: I haz the power, 2012-01-19 16:29:33]

Change committed as 118303