Fixes bug where chrome hangs in Mac when clicking on Permission link on Extensions.

Fixes bug where chrome hangs in Mac when clicking on Permission link on Extensions.

RootCause: Permission Prompt does not have a OK button. And Chrome 
hangs while trying to create a OK button with empty string.

Solution: Remove |okButton_| when its title is empty and adjust 
|cancelButton_|'s position.
The screenshot with the fix is uploaded at http://crbug.com/175071#c15

This CL follows http://crrev.com/12220142/

BUG=175071

[Joe Thomas, 2013-02-13 22:31:55]

Please take a look.

[Matt Perry, 2013-02-13 23:36:52]

OK, LGTM.

If you're sure this fixes the issue, please also revert
http://src.chromium.org/viewvc/chrome?view=rev&revision=182274 , which reverted
the permissions link code.

Also, please update the CL description. You want something like "Fixes bug where
<bug>".

[Joe Thomas, 2013-02-13 23:47:12]

+ sail for OWNERS

[Joe Thomas, 2013-02-13 23:48:11]

On 2013/02/13 23:36:52, Matt Perry wrote:
> OK, LGTM.
> 
> If you're sure this fixes the issue, please also revert
> http://src.chromium.org/viewvc/chrome?view=rev&revision=182274 , which
reverted
> the permissions link code.
> 
> Also, please update the CL description. You want something like "Fixes bug
where
> <bug>".

Thanks for the review. Corrected the CL description.

[sail, 2013-02-13 23:56:36]

Your CL description mentions permission link and hang. The CL is about labels on
buttons. One of those two should change.

The CL needs a regression test (a test that fails without your fix and passes
with it).

The CL description needs a link to a screenshot with your new UI.

https://codereview.chromium.org/12258023/diff/1/chrome/browser/ui/cocoa/exten...
File chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
(right):

https://codereview.chromium.org/12258023/diff/1/chrome/browser/ui/cocoa/exten...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:240:
[cancelButton_ setFrame:okButtonRect];
I don't think this will work. For example, if the cancel label is really long
and the ok button rect causes it to become clipped.

You probably want something like this:
  NSRect cancelButtonRect = [cancelButton_ frame];
  cancelButton.origin.x = NSMaxX(okButtonRect) - NSWidth(cancelButtonRect);
  [cancelButton_ setFrame:cancelButton]

[Joe Thomas, 2013-02-14 02:41:59]

Thanks for the review. Patch updated. Please take another look.


>Your CL description mentions permission link and hang. The CL is about >labels
on
>buttons. One of those two should change.

Updated the CL description with root cause.

>The CL needs a regression test (a test that fails without your fix and passes
>with it).

Done.

>The CL description needs a link to a screenshot with your new UI.

http://code.google.com/p/chromium/issues/detail?id=175071#c15

https://codereview.chromium.org/12258023/diff/1/chrome/browser/ui/cocoa/exten...
File chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm
(right):

https://codereview.chromium.org/12258023/diff/1/chrome/browser/ui/cocoa/exten...
chrome/browser/ui/cocoa/extensions/extension_install_view_controller.mm:240:
[cancelButton_ setFrame:okButtonRect];
On 2013/02/13 23:56:37, sail wrote:
> I don't think this will work. For example, if the cancel label is really long
> and the ok button rect causes it to become clipped.
> 
> You probably want something like this:
>   NSRect cancelButtonRect = [cancelButton_ frame];
>   cancelButton.origin.x = NSMaxX(okButtonRect) - NSWidth(cancelButtonRect);
>   [cancelButton_ setFrame:cancelButton]

Done.

[sail, 2013-02-14 16:54:04]

LGTM
The link to the screenshot needs to go in the CL description.

[Joe Thomas, 2013-02-14 22:42:12]

This CL is combined with http://crrev.com/12212195

[Finnur, 2013-02-15 10:21:22]

Thank you, Joe, for picking this up. Greatly appreciated.

[Joe Thomas, 2013-02-15 20:48:34]

On 2013/02/15 10:21:22, Finnur wrote:
> Thank you, Joe, for picking this up. Greatly appreciated.

You're welcome!

Closing this CL as the changes landed with http://crrev.com/12212195/