WebUI TaskManager: Use the specified height of row on refleshing.

WebUI TaskManager: Use the specified height of row on refleshing. 

Measuring of element is expensive operation. This CL prevents to measurs of size of row if the height is specified explicitly on CSS. It saves time of size calculation of the element. 

In the previous implementation, the size (including both height and width) was cached but the width has been not used. This CL changes to cache only the height and to remove unused width-related code. 

BUG=99029
TEST=manual

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

[yoshiki, 2012-01-25 10:19:21]

mazda@ for review, and jhawkins@ for approval.

[James Hawkins, 2012-01-26 01:34:05]

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:386: * @param {ListItem=} item
The list item to use to do the measuring.
"The list item to measure."

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:393: // Uses the fixed height
if set it on CSS, to save a time of rayout
s/rayout/layout/

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:995: var height =
this.measureItemHeight_(measuringItems[y]);
No need to temp variable, just assign directly.

[mazda, 2012-01-26 09:37:16]

http://codereview.chromium.org/9288017/diff/1/chrome/browser/resources/shared...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

http://codereview.chromium.org/9288017/diff/1/chrome/browser/resources/shared...
chrome/browser/resources/shared/js/cr/ui/list.js:388: * is set, uses that value.
Otherwise, measures the size.
Please note that the fixed height is used only if it's set by 'px'.

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

http://codereview.chromium.org/9288017/diff/1/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:501: listItem.style.height =
(data['uniqueId'].length * 20) + 'px';
What is this change intended for?

[yoshiki, 2012-01-26 10:10:45]

PTAL

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:386: * @param {ListItem=} item
The list item to use to do the measuring.
On 2012/01/26 01:34:05, James Hawkins wrote:
> "The list item to measure."

Done.

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:388: * is set, uses that value.
Otherwise, measures the size.
On 2012/01/26 09:37:16, mazda wrote:
> Please note that the fixed height is used only if it's set by 'px'.

Done.

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:393: // Uses the fixed height
if set it on CSS, to save a time of rayout
On 2012/01/26 01:34:05, James Hawkins wrote:
> s/rayout/layout/

Done.

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/shared/js/cr/ui/list.js:995: var height =
this.measureItemHeight_(measuringItems[y]);
On 2012/01/26 01:34:05, James Hawkins wrote:
> No need to temp variable, just assign directly.

Done.

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
File chrome/browser/resources/task_manager/main.js (right):

https://chromiumcodereview.appspot.com/9288017/diff/1/chrome/browser/resource...
chrome/browser/resources/task_manager/main.js:501: listItem.style.height =
(data['uniqueId'].length * 20) + 'px';
I added an additional information as comment. Is it clear?

On 2012/01/26 09:37:16, mazda wrote:
> What is this change intended for?

[mazda, 2012-01-26 11:20:15]

lgtm with a comment.

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

http://codereview.chromium.org/9288017/diff/6/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:503: // 'num_of_tasks * 20' px.
Please make a variable for 20.

[James Hawkins, 2012-01-26 17:08:56]

LGTM with nit (and mazda's comment).

http://codereview.chromium.org/9288017/diff/6/chrome/browser/resources/shared...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

http://codereview.chromium.org/9288017/diff/6/chrome/browser/resources/shared...
chrome/browser/resources/shared/js/cr/ui/list.js:393: // Uses the fixed height
if set it on CSS, to save a time of layout
nit: s/Uses/Use/

[yoshiki, 2012-01-31 11:30:35]

Thanks. Send it to commit queue!

http://codereview.chromium.org/9288017/diff/6/chrome/browser/resources/shared...
File chrome/browser/resources/shared/js/cr/ui/list.js (right):

http://codereview.chromium.org/9288017/diff/6/chrome/browser/resources/shared...
chrome/browser/resources/shared/js/cr/ui/list.js:393: // Uses the fixed height
if set it on CSS, to save a time of layout
On 2012/01/26 17:08:57, James Hawkins wrote:
> nit: s/Uses/Use/

Done.

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

http://codereview.chromium.org/9288017/diff/6/chrome/browser/resources/task_m...
chrome/browser/resources/task_manager/main.js:503: // 'num_of_tasks * 20' px.
On 2012/01/26 11:20:15, mazda wrote:
> Please make a variable for 20.

Done.

[commit-bot: I haz the power, 2012-01-31 11:30:46]

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

[commit-bot: I haz the power, 2012-01-31 12:51:27]

Change committed as 119890