Strip query string before applying url mapping.

Strip query string before applying url mapping.

The query string is now the state of an application. If url1 is mapped
to url2, then url1?state should be mapped to url2?state. This CL ensures
this behavior.

R=davemoore@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/05eccab81b40205a5c90eb86da38bdf921582202

[qsr, 2015-03-04 09:34:31]

gentle ping?

[DaveMoore, 2015-03-04 14:57:07]

https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
File shell/application_manager/query_util.h (right):

https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
shell/application_manager/query_util.h:15: GURL StripQueryFromURL(const GURL&
url);
Instead of having two methods here why not just

GURL GetBaseURLAndQuery(const GURL& url, std::string* query)

where query can be NULL.

[qsr, 2015-03-04 15:02:05]

https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
File shell/application_manager/query_util.h (right):

https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
shell/application_manager/query_util.h:15: GURL StripQueryFromURL(const GURL&
url);
On 2015/03/04 14:57:07, DaveMoore wrote:
> Instead of having two methods here why not just
> 
> GURL GetBaseURLAndQuery(const GURL& url, std::string* query)
> 
> where query can be NULL.

Mainly because I didn't want to have some nullptr in most call sites, as this
method is used a lot more than the other. WDYT?

[DaveMoore, 2015-03-04 16:00:57]

On 2015/03/04 15:02:05, qsr wrote:
>
https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
> File shell/application_manager/query_util.h (right):
> 
>
https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
> shell/application_manager/query_util.h:15: GURL StripQueryFromURL(const GURL&
> url);
> On 2015/03/04 14:57:07, DaveMoore wrote:
> > Instead of having two methods here why not just
> > 
> > GURL GetBaseURLAndQuery(const GURL& url, std::string* query)
> > 
> > where query can be NULL.
> 
> Mainly because I didn't want to have some nullptr in most call sites, as this
> method is used a lot more than the other. WDYT?

I would still rather have one method than two. Or have one that gets the base
url and a different one that gets the query string. Or keep the one that returns
both as a pair<> and have the callers extract the URL from it. 

If you insist on keeping two methods then they at least should have consistent
naming. Right now one is Split... and the other is Get...  Plus the Split... one
doesn't split, it returns only part.

[qsr, 2015-03-04 16:10:58]

On 2015/03/04 16:00:57, DaveMoore wrote:
> On 2015/03/04 15:02:05, qsr wrote:
> >
>
https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
> > File shell/application_manager/query_util.h (right):
> > 
> >
>
https://codereview.chromium.org/972473003/diff/1/shell/application_manager/qu...
> > shell/application_manager/query_util.h:15: GURL StripQueryFromURL(const
GURL&
> > url);
> > On 2015/03/04 14:57:07, DaveMoore wrote:
> > > Instead of having two methods here why not just
> > > 
> > > GURL GetBaseURLAndQuery(const GURL& url, std::string* query)
> > > 
> > > where query can be NULL.
> > 
> > Mainly because I didn't want to have some nullptr in most call sites, as
this
> > method is used a lot more than the other. WDYT?
> 
> I would still rather have one method than two. Or have one that gets the base
> url and a different one that gets the query string. Or keep the one that
returns
> both as a pair<> and have the callers extract the URL from it. 
> 
> If you insist on keeping two methods then they at least should have consistent
> naming. Right now one is Split... and the other is Get...  Plus the Split...
one
> doesn't split, it returns only part.

 I don't insist. Will gladly change. Just wanted to make sure you didn't mind
the nullptr being used at multiple places.

[qsr, 2015-03-04 16:22:19]

Updated.

[DaveMoore, 2015-03-04 16:38:17]

lgtm

[qsr, 2015-03-05 14:29:42]

Committed patchset #2 (id:20001) manually as
05eccab81b40205a5c90eb86da38bdf921582202 (presubmit successful).