Upstream "nsurl_util.{h,mm}" from Chrome on iOS repository.

Upstream "nsurl_util.{h,mm}" from Chrome on iOS repository.

I created the new directory net/base/mac/ to hold shared files between OSX
and iOS. I moved the files from "net/ios/" (Chrome on iOS repository) to
"net/base/mac/".

BUG=

[erikchen, 2014-11-12 23:06:00]

Patchset #2 (id:20001) has been deleted

[erikchen, 2014-11-12 23:12:27]

Patchset #2 (id:40001) has been deleted

[erikchen, 2014-11-12 23:15:33]

Patchset #2 (id:60001) has been deleted

[erikchen, 2014-11-12 23:19:13]

erikchen@chromium.org changed reviewers:
+ droger@chromium.org, ellyjones@chromium.org

[erikchen, 2014-11-12 23:19:14]

ellyjones, droger: Please review.

[droger, 2014-11-13 12:29:04]

I think net/mac_ios is not the right place, and net/mac would be better.

My understanding is that iOS currently uses code from mac/ subdirectories in a
number of places, but not the other way around. And thus mac/ is the place to
put shared code.

Looking at these file, I wonder if this would fit better in //url rather than
//net, i.e.
//url/mac/url_conversions.h or something along those lines?

[erikchen, 2014-11-13 18:23:21]

Patchset #3 (id:100001) has been deleted

[erikchen, 2014-11-13 18:39:14]

On 2014/11/13 12:29:04, droger wrote:
> I think net/mac_ios is not the right place, and net/mac would be better.
> 
> My understanding is that iOS currently uses code from mac/ subdirectories in a
> number of places, but not the other way around. And thus mac/ is the place to
> put shared code.
> 
> Looking at these file, I wonder if this would fit better in //url rather than
> //net, i.e.
> //url/mac/url_conversions.h or something along those lines?

droger: PTAL

I've recently had troubles with /mac/ directories in iOS due to the awkward
filename_rules used on iOS. That being said, you're right, /mac/ is the right
folder name for shared files betwen iOS and Mac :(. I've moved the files to
url/mac/url_conversions* as you suggested.

[droger, 2014-11-13 23:32:37]

Lgtm

Please discuss with an owner of //url to confirm that this is the right place
for this code.

Note: I did not review the code in details, because this is upstreaming of code
that was already internally reviewed before, and I assumed you did not change it
significantly.

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions.mm
File url/mac/url_conversions.mm (right):

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions...
url/mac/url_conversions.mm:45: // These are all the characters that NSURL
*requies* to be encoded throughout.
requires

[droger, 2014-11-13 23:37:12]

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions.h
File url/mac/url_conversions.h (right):

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions...
url/mac/url_conversions.h:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
Nit: Not sure if this is important, but we should probably keep the original
year for the files (I did not check but I would think that this file is older
than 2014).

[erikchen, 2014-11-13 23:41:25]

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions.h
File url/mac/url_conversions.h (right):

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions...
url/mac/url_conversions.h:1: // Copyright 2014 The Chromium Authors. All rights
reserved.
On 2014/11/13 23:37:12, droger wrote:
> Nit: Not sure if this is important, but we should probably keep the original
> year for the files (I did not check but I would think that this file is older
> than 2014).

Done.

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions.mm
File url/mac/url_conversions.mm (right):

https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions...
url/mac/url_conversions.mm:45: // These are all the characters that NSURL
*requies* to be encoded throughout.
On 2014/11/13 23:32:36, droger wrote:
> requires

Done.

[erikchen, 2014-11-13 23:41:49]

On 2014/11/13 23:32:37, droger wrote:
> Lgtm
> 
> Please discuss with an owner of //url to confirm that this is the right place
> for this code.
> 
> Note: I did not review the code in details, because this is upstreaming of
code
> that was already internally reviewed before, and I assumed you did not change
it
> significantly.
> 
>
https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions.mm
> File url/mac/url_conversions.mm (right):
> 
>
https://codereview.chromium.org/719783005/diff/140001/url/mac/url_conversions...
> url/mac/url_conversions.mm:45: // These are all the characters that NSURL
> *requies* to be encoded throughout.
> requires

Correct. If you diff patch set 4 against patch set 3, you can see that my
changes are minimal.

[erikchen, 2014-11-13 23:43:33]

erikchen@chromium.org changed reviewers:
+ abarth@chromium.org

[erikchen, 2014-11-13 23:43:34]

abarth: Looking for an OWNER review of url/. Also, please confirm that url/ is
the right location to upstream this file from the iOS repository.

Diff patch set 5 against patch set 3 to see the changes I made to the files
being upstreamed.

[abarth-chromium, 2014-11-14 03:36:40]

I don't think //url is the right place for this code.  It seems more appropriate
for some library that uses //url.

[droger, 2014-11-14 09:04:38]

On 2014/11/14 03:36:40, abarth wrote:
> I don't think //url is the right place for this code.  It seems more
appropriate
> for some library that uses //url.

I'm curious to know why.

Here are some arguments why I suggested //url as a place for this code:
- this code is needed by //net/ios and thus has to be in //net or below. The
obvious candidates are thus //net and //url.
- this is not really network code, but looks much more like url code
- looking at what is done in //base, I can see that there are a lot of
conversion utils for mac (for example between base::string16 and NSString). It
seems to me that putting conversions between GURL and NSURL would be the
corresponding pattern in //url.

Do you have suggestions where we could put this code?

[erikchen, 2014-11-18 19:08:12]

abarth: Ping?

[erikchen, 2014-11-18 19:08:13]

abarth: Ping?

[abarth-chromium, 2014-11-20 22:29:19]

not lgtm

//url is just a URL parsing library.  It's not its job to integrate with other
URL parsing libraries.  For example, why not put this code in NSURL?  GURL and
NSURL are peers in this respect.  I understand that you don't have the ability
to add code to NSURL, but that's not a reason to add this code to GURL.  Please
put it somewhere else.

[erikchen, 2014-11-21 01:58:09]

Patchset #7 (id:200001) has been deleted

[erikchen, 2014-11-21 02:03:47]

Patchset #7 (id:220001) has been deleted

[erikchen, 2014-11-21 02:04:56]

Patchset #7 (id:240001) has been deleted

[erikchen, 2014-11-21 02:08:54]

On 2014/11/20 22:29:19, abarth wrote:
> not lgtm
> 
> //url is just a URL parsing library.  It's not its job to integrate with other
> URL parsing libraries.  For example, why not put this code in NSURL?  GURL and
> NSURL are peers in this respect.  I understand that you don't have the ability
> to add code to NSURL, but that's not a reason to add this code to GURL. 
Please
> put it somewhere else.

Closing this CL. Made a clone of it at
https://codereview.chromium.org/747773002/, where the files have instead been
moved to net/base/mac/.

[erikchen, 2014-11-21 02:10:03]

Patchset #6 (id:180001) has been deleted