Allow resizing a panel using chrome.windows.update() extension API.

Allow resizing a panel using chrome.windows.update() extension API.

BUG=108222
TEST=new extension api test added


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

[jennb, 2012-01-10 23:53:38]

Got time to review this? Thx.

[jianli, 2012-01-11 00:29:12]

http://codereview.chromium.org/9181005/diff/1/chrome/browser/ui/panels/panel.cc
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/9181005/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.cc:179: manager()->ResizePanel(this,
bounds.size());
For detached panel case, do we want to allow the API to update the position?

http://codereview.chromium.org/9181005/diff/1/chrome/test/data/extensions/api...
File chrome/test/data/extensions/api_test/window_update/resize/test.js (right):

http://codereview.chromium.org/9181005/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/window_update/resize/test.js:7: var
widthDelta = 50;
Perhaps we also want to add the test cases to cover decreasing the width and
height.

http://codereview.chromium.org/9181005/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/window_update/resize/test.js:31: function
checkWidthAndContinue(currentWindow) {
This function also updates the height in addition to checking width. So the
function name does not tell all of things done in this function. How about sth.
like the following:

var testCases = [
  "updateWidth",
  "updateHeight",
  "updateWidthAndHeight",
];
var testIndex = 0;

function updateWidth(currentWindow) {
  expectedWidth = currentWindow.width + widthDelta;
  expectedHeight = currentWindow.height;
  chrome.windows.update(
    currentWindow.id, { 'width': expectedWidth },
    pass(checkUpdate)
  );
}

function updateHeight(currentWindow) {
  ...
}

function updateWidthAndHeight(currentWindow) {
  ...
}

function runNextTest(currentWindow) {
  if (testIndex < testCases.length) {
    testIndex++;
    self[testCases[testIndex - 1]](currentWindow);
  }
}

function checkUpdate(currentWindow) {
  chrome.test.assertEq(expectedWidth, currentWindow.width);
  chrome.test.assertEq(expectedHeight, currentWindow.height);
  runNextTest(currentWindow);
}

[jennb, 2012-01-11 00:36:26]

http://codereview.chromium.org/9181005/diff/1/chrome/browser/ui/panels/panel.cc
File chrome/browser/ui/panels/panel.cc (right):

http://codereview.chromium.org/9181005/diff/1/chrome/browser/ui/panels/panel....
chrome/browser/ui/panels/panel.cc:179: manager()->ResizePanel(this,
bounds.size());
On 2012/01/11 00:29:13, jianli wrote:
> For detached panel case, do we want to allow the API to update the position?

Probably, but that's future code.

http://codereview.chromium.org/9181005/diff/1/chrome/test/data/extensions/api...
File chrome/test/data/extensions/api_test/window_update/resize/test.js (right):

http://codereview.chromium.org/9181005/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/window_update/resize/test.js:7: var
widthDelta = 50;
On 2012/01/11 00:29:13, jianli wrote:
> Perhaps we also want to add the test cases to cover decreasing the width and
> height.

Code path is the same regardless if the delta is positive or negative so we
wouldn't really gain added test coverage by doing so.

http://codereview.chromium.org/9181005/diff/1/chrome/test/data/extensions/api...
chrome/test/data/extensions/api_test/window_update/resize/test.js:31: function
checkWidthAndContinue(currentWindow) {
On 2012/01/11 00:29:13, jianli wrote:
> This function also updates the height in addition to checking width. So the
> function name does not tell all of things done in this function. How about
sth.
> like the following:
> 
> var testCases = [
>   "updateWidth",
>   "updateHeight",
>   "updateWidthAndHeight",
> ];
> var testIndex = 0;
> 
> function updateWidth(currentWindow) {
>   expectedWidth = currentWindow.width + widthDelta;
>   expectedHeight = currentWindow.height;
>   chrome.windows.update(
>     currentWindow.id, { 'width': expectedWidth },
>     pass(checkUpdate)
>   );
> }
> 
> function updateHeight(currentWindow) {
>   ...
> }
> 
> function updateWidthAndHeight(currentWindow) {
>   ...
> }
> 
> function runNextTest(currentWindow) {
>   if (testIndex < testCases.length) {
>     testIndex++;
>     self[testCases[testIndex - 1]](currentWindow);
>   }
> }
> 
> function checkUpdate(currentWindow) {
>   chrome.test.assertEq(expectedWidth, currentWindow.width);
>   chrome.test.assertEq(expectedHeight, currentWindow.height);
>   runNextTest(currentWindow);
> }
> 

The test code follows the pattern in window_update/sizing.

[jianli, 2012-01-11 01:27:25]

lgtm

[commit-bot: I haz the power, 2012-01-11 18:01:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jennb@chromium.org/9181005/9001

[commit-bot: I haz the power, 2012-01-11 20:07:53]

Change committed as 117269