Bulletproof the url going into a std::string and ensure it's not going to...

Bulletproof the url going into a std::string and ensure it's not going to
be NULL, which throws and exception.
BUG=26883
TEST=dragging things into the content area (urls, images, text, etc)

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

[viettrungluu, 2009-11-06 21:29:17]

LGTM with comment below addressed.

http://codereview.chromium.org/373016/diff/1/2
File chrome/browser/cocoa/web_drop_target.mm (right):

http://codereview.chromium.org/373016/diff/1/2#newcode196
Line 196: data->url_title = base::SysNSStringToUTF16([titles objectAtIndex:0]);
I'm 99.99% sure that |SysNSStringToUTF16()| chokes if passed nil, so this should
be something like:

  if (NSString* title = [titles objectAtIndex:0])
    data->url_title = base::SysNSStringToUTF16(title);

(|-objectAtIndex:| shouldn't return nil, but is there any guarantee that
|titles| isn't nil?)

[pink (ping after 24hrs), 2009-11-06 21:34:25]

We're fixing the SysNSString* calls to accept nil and return empty
string as part of a cl that's currently ongoing.

On Fri, Nov 6, 2009 at 4:29 PM,  <viettrungluu@chromium.org> wrote:
> LGTM with comment below addressed.
>
>
> http://codereview.chromium.org/373016/diff/1/2
> File chrome/browser/cocoa/web_drop_target.mm (right):
>
> http://codereview.chromium.org/373016/diff/1/2#newcode196
> Line 196: data->url_title =3D base::SysNSStringToUTF16([titles
> objectAtIndex:0]);
> I'm 99.99% sure that |SysNSStringToUTF16()| chokes if passed nil, so
> this should be something like:
>
> =A0if (NSString* title =3D [titles objectAtIndex:0])
> =A0 =A0data->url_title =3D base::SysNSStringToUTF16(title);
>
> (|-objectAtIndex:| shouldn't return nil, but is there any guarantee that
> |titles| isn't nil?)
>
> http://codereview.chromium.org/373016
>



--=20
Mike Pinkerton
Mac Weenie
pinkerton@google.com

[viettrungluu, 2009-11-06 21:36:50]

Okay, LGTM on this change. Do we have a bug filed for the SysNSString* calls to
make sure it doesn't get lost in the shuffle?

On 2009/11/06 21:34:25, pink wrote:
> We're fixing the SysNSString* calls to accept nil and return empty
> string as part of a cl that's currently ongoing.
> 
> On Fri, Nov 6, 2009 at 4:29 PM,  <mailto:viettrungluu@chromium.org> wrote:
> > LGTM with comment below addressed.
> >
> >
> > http://codereview.chromium.org/373016/diff/1/2
> > File chrome/browser/cocoa/web_drop_target.mm (right):
> >
> > http://codereview.chromium.org/373016/diff/1/2#newcode196
> > Line 196: data->url_title =3D base::SysNSStringToUTF16([titles
> > objectAtIndex:0]);
> > I'm 99.99% sure that |SysNSStringToUTF16()| chokes if passed nil, so
> > this should be something like:
> >
> > =A0if (NSString* title =3D [titles objectAtIndex:0])
> > =A0 =A0data->url_title =3D base::SysNSStringToUTF16(title);
> >
> > (|-objectAtIndex:| shouldn't return nil, but is there any guarantee that
> > |titles| isn't nil?)
> >
> > http://codereview.chromium.org/373016
> >
> 
> 
> 
> --=20
> Mike Pinkerton
> Mac Weenie
> mailto:pinkerton@google.com