mac: Switch to "new" (10.6) block-based panel apis

mac: Switch to "new" (10.6) block-based panel apis

The new APIs enable saving of general NSURLs. Strip NSURLs that
aren't isFileURLs.

The old apis are deprecated in the 10.8 sdk.

BUG=139138
TBR=sky

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

[Avi (use Gerrit), 2012-07-27 14:36:01]

LGTM with the refactor.

http://codereview.chromium.org/10829053/diff/1/ui/base/dialogs/select_file_di...
File ui/base/dialogs/select_file_dialog_mac.mm (right):

http://codereview.chromium.org/10829053/diff/1/ui/base/dialogs/select_file_di...
ui/base/dialogs/select_file_dialog_mac.mm:268: }];
This code (from -setDirectoryURL down to here)...

http://codereview.chromium.org/10829053/diff/1/ui/base/dialogs/select_file_di...
ui/base/dialogs/select_file_dialog_mac.mm:298: }];
...and this code here are identical. Consolidate them outside the if.

[Nico, 2012-07-27 14:48:43]

Thanks!

 1 file changed, 25 insertions(+), 52 deletions(-)

http://codereview.chromium.org/10829053/diff/1/ui/base/dialogs/select_file_di...
File ui/base/dialogs/select_file_dialog_mac.mm (right):

http://codereview.chromium.org/10829053/diff/1/ui/base/dialogs/select_file_di...
ui/base/dialogs/select_file_dialog_mac.mm:298: }];
On 2012/07/27 14:36:01, Avi wrote:
> ...and this code here are identical. Consolidate them outside the if.

Done.

[Nico, 2012-07-27 14:54:45]

Did another minor change, now it crashes less than the previous patch sets :-P

[commit-bot: I haz the power, 2012-07-27 14:57:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/10829053/4002

[commit-bot: I haz the power, 2012-07-27 14:59:00]

Presubmit check for 10829053-4002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    ui

Presubmit checks took 30.7s to calculate.

[Avi (use Gerrit), 2012-07-27 15:01:21]

http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
File ui/base/dialogs/select_file_dialog_mac.mm (right):

http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
ui/base/dialogs/select_file_dialog_mac.mm:292: }];
Indentation is wrong here; closing braces match the indentation of the line on
which the block was opened. See the style guide.

[Nico, 2012-07-27 15:08:19]

sky: OWNERS

[commit-bot: I haz the power, 2012-07-27 15:11:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/10829053/4002

[Nico, 2012-07-27 15:14:40]

On Fri, Jul 27, 2012 at 8:01 AM,  <avi@chromium.org> wrote:
>
>
http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
> File ui/base/dialogs/select_file_dialog_mac.mm (right):
>
>
http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
> ui/base/dialogs/select_file_dialog_mac.mm:292: }];
> Indentation is wrong here; closing braces match the indentation of the
> line on which the block was opened. See the style guide.

"""// The block can be put on a new line, indented four spaces, with the
// closing brace aligned with the first character of the line on which
// block was declared.
[operation setCompletionBlock:^{
    [self.delegate newDataAvailable];
}];
"""

[Nico, 2012-07-27 15:14:58]

On Fri, Jul 27, 2012 at 8:14 AM, Nico Weber <thakis@chromium.org> wrote:
> On Fri, Jul 27, 2012 at 8:01 AM,  <avi@chromium.org> wrote:
>>
>>
http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
>> File ui/base/dialogs/select_file_dialog_mac.mm (right):
>>
>>
http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
>> ui/base/dialogs/select_file_dialog_mac.mm:292: }];
>> Indentation is wrong here; closing braces match the indentation of the
>> line on which the block was opened. See the style guide.
>
> """// The block can be put on a new line, indented four spaces, with the
> // closing brace aligned with the first character of the line on which
> // block was declared.
> [operation setCompletionBlock:^{
>     [self.delegate newDataAvailable];
> }];
> """

I fail at reading.

[Nico, 2012-07-27 15:16:49]

On Fri, Jul 27, 2012 at 8:14 AM, Nico Weber <thakis@chromium.org> wrote:
> On Fri, Jul 27, 2012 at 8:14 AM, Nico Weber <thakis@chromium.org> wrote:
>> On Fri, Jul 27, 2012 at 8:01 AM,  <avi@chromium.org> wrote:
>>>
>>>
http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
>>> File ui/base/dialogs/select_file_dialog_mac.mm (right):
>>>
>>>
http://codereview.chromium.org/10829053/diff/4002/ui/base/dialogs/select_file...
>>> ui/base/dialogs/select_file_dialog_mac.mm:292: }];
>>> Indentation is wrong here; closing braces match the indentation of the
>>> line on which the block was opened. See the style guide.
>>
>> """// The block can be put on a new line, indented four spaces, with the
>> // closing brace aligned with the first character of the line on which
>> // block was declared.
>> [operation setCompletionBlock:^{
>>     [self.delegate newDataAvailable];
>> }];
>> """
>
> I fail at reading.

But this looks seriously weird:

  [dialog beginSheetModalForWindow:owning_window
                 completionHandler:^(NSInteger result) {
    [bridge_.get() endedPanel:dialog
                    didCancel:result != NSFileHandlingPanelOKButton
                         type:type
                 parentWindow:owning_window];
                 }];

I'll keep it as is.

[Avi (use Gerrit), 2012-07-27 20:04:22]

It looks weird because your indent starting on the line [bridge_.get() is wrong.
But you can't fix that without running afoul of 80 characters.

Certainly not withholding an LG over that, though.

[commit-bot: I haz the power, 2012-07-27 20:18:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thakis@chromium.org/10829053/4002

[commit-bot: I haz the power, 2012-07-27 21:36:08]

Change committed as 148818