AddOriginMapping()

AddOriginMapping()

BUG=
R=aa@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/23538d3c49dfaff28e4d88b3254b7ab8cf1b6e35

[DaveMoore, 2015-02-13 22:28:17]

Comments

[DaveMoore, 2015-02-13 22:29:35]

davemoore@chromium.org changed reviewers:
+ aa@chromium.org, blundell@chromium.org

[Aaron Boodman, 2015-02-13 22:44:16]

lgtm

https://codereview.chromium.org/926093003/diff/20001/shell/switches.cc
File shell/switches.cc (right):

https://codereview.chromium.org/926093003/diff/20001/shell/switches.cc#newcode59
shell/switches.cc:59: // first maps 'a' to 'b' and the second 'c' to 'd'.
Are you going to make this match the style of map-origin in a separate change?

https://codereview.chromium.org/926093003/diff/20001/shell/url_resolver.cc
File shell/url_resolver.cc (right):

https://codereview.chromium.org/926093003/diff/20001/shell/url_resolver.cc#ne...
shell/url_resolver.cc:30: "-" + std::string(switches::kMapOrigin) + "=",
Can't we just pick one? Why have choices when we can not have choices?

[DaveMoore, 2015-02-13 23:07:10]

https://codereview.chromium.org/926093003/diff/20001/shell/switches.cc
File shell/switches.cc (right):

https://codereview.chromium.org/926093003/diff/20001/shell/switches.cc#newcode59
shell/switches.cc:59: // first maps 'a' to 'b' and the second 'c' to 'd'.
On 2015/02/13 22:44:15, Aaron Boodman wrote:
> Are you going to make this match the style of map-origin in a separate change?

Yes.

https://codereview.chromium.org/926093003/diff/20001/shell/url_resolver.cc
File shell/url_resolver.cc (right):

https://codereview.chromium.org/926093003/diff/20001/shell/url_resolver.cc#ne...
shell/url_resolver.cc:30: "-" + std::string(switches::kMapOrigin) + "=",
On 2015/02/13 22:44:15, Aaron Boodman wrote:
> Can't we just pick one? Why have choices when we can not have choices?

It's consistent with the rest of the switches. base::CommandLine does this as
well.

[DaveMoore, 2015-02-13 23:11:00]

New patchsets have been uploaded after l-g-t-m from aa@chromium.org

[DaveMoore, 2015-02-13 23:11:01]

Add copy constructor

[DaveMoore, 2015-02-13 23:14:16]

Try removing const

[DaveMoore, 2015-02-13 23:23:50]

Committed patchset #4 (id:60001) manually as
23538d3c49dfaff28e4d88b3254b7ab8cf1b6e35 (presubmit successful).

[jamesr, 2015-02-14 00:47:38]

jamesr@chromium.org changed reviewers:
+ jamesr@chromium.org

[jamesr, 2015-02-14 00:47:39]

https://codereview.chromium.org/926093003/diff/60001/services/console/console.cc
File services/console/console.cc (right):

https://codereview.chromium.org/926093003/diff/60001/services/console/console...
services/console/console.cc:19: class ConsoleImpl : public
InterfaceImpl<Console> {
I just fixed this yesterday, why are you reverting it back?

[jamesr, 2015-02-14 00:50:32]

Looks like you probably didn't intend to patch console at all.  I'll revert that
bit.

[blundell, 2015-02-16 08:32:34]

Thanks for doing this, Dave!

https://codereview.chromium.org/926093003/diff/20001/shell/url_resolver.cc
File shell/url_resolver.cc (right):

https://codereview.chromium.org/926093003/diff/20001/shell/url_resolver.cc#ne...
shell/url_resolver.cc:30: "-" + std::string(switches::kMapOrigin) + "=",
nit: is it not possible to allow a space instead of an "=" here as well?