Don't pass class types across EXE/DLL boundaries for SetCommandLine() and SetExperimentList().

Don't pass class types across EXE/DLL boundaries for SetCommandLine() and SetExperimentList().

BUG=128541
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138353

[eroman, 2012-05-21 22:22:42]

@mad: Please review changes to SetExperimentList()
@cpu: Please review changes to SetCommandLine()

[cpu_(ooo_6.6-7.5), 2012-05-22 02:30:33]

lgtm

[MAD, 2012-05-22 14:30:49]

LGTM2... Thanks a lot... Well done...

I have one small request for an augmented comment though...

BYE
MAD

http://codereview.chromium.org/10411059/diff/2001/chrome/app/breakpad_win.h
File chrome/app/breakpad_win.h (right):

http://codereview.chromium.org/10411059/diff/2001/chrome/app/breakpad_win.h#n...
chrome/app/breakpad_win.h:45: // Helper to convert a vector of wstrings to
corresponding vector of cstrings.
Please add a comment that the cstrings vector can't outlive wstrings...

[eroman, 2012-05-22 20:33:28]

http://codereview.chromium.org/10411059/diff/2001/chrome/app/breakpad_win.h
File chrome/app/breakpad_win.h (right):

http://codereview.chromium.org/10411059/diff/2001/chrome/app/breakpad_win.h#n...
chrome/app/breakpad_win.h:45: // Helper to convert a vector of wstrings to
corresponding vector of cstrings.
On 2012/05/22 14:30:49, MAD wrote:
> Please add a comment that the cstrings vector can't outlive wstrings... 

Done.

[commit-bot: I haz the power, 2012-05-22 20:35:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/eroman@chromium.org/10411059/5002

[commit-bot: I haz the power, 2012-05-22 20:35:58]

Presubmit check for 10411059-5002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit Warnings **
New code should not use wstrings.  If you are calling an API that accepts a
wstring, fix the API.
    chrome/app/breakpad_win.cc:828
    chrome/app/breakpad_win.cc:831
    chrome/app/breakpad_win.cc:832
    chrome/app/breakpad_win.cc:833
    chrome/app/breakpad_win.h:45
    chrome/app/breakpad_win.h:46
    chrome/app/breakpad_win.h:47
    chrome/app/breakpad_win.h:48
    chrome/common/child_process_logging_win.cc:59
    chrome/common/child_process_logging_win.cc:62
    chrome/common/child_process_logging_win.cc:63
    chrome/common/child_process_logging_win.cc:64

Presubmit checks took 1.4s to calculate.

[eroman, 2012-05-22 20:38:32]

I don't think this warning is legitimate, since this is Windows-only code.

I can easily rename to string16 later if that is the desired practice, but I'm
going to commit as-is for now to get the fix in.

[MAD, 2012-05-22 20:40:46]

Agreed...

BYE
MAD...
sent from Mobile Answer Device!
Le 22 mai 2012 16:38, <eroman@chromium.org> a écrit :

> I don't think this warning is legitimate, since this is Windows-only code.
>
> I can easily rename to string16 later if that is the desired practice, but
> I'm
> going to commit as-is for now to get the fix in.
>
>
http://codereview.chromium.**org/10411059/<http://codereview.chromium.org/104...
>

[MAD, 2012-05-22 20:41:42]

Still lgtm... :-)

BYE
MAD...
sent from Mobile Answer Device!
Le 22 mai 2012 16:40, "Marc-Andre Decoste" <mad@chromium.org> a écrit :

> Agreed...
>
> BYE
> MAD...
> sent from Mobile Answer Device!
> Le 22 mai 2012 16:38, <eroman@chromium.org> a écrit :
>
>> I don't think this warning is legitimate, since this is Windows-only code.
>>
>> I can easily rename to string16 later if that is the desired practice,
>> but I'm
>> going to commit as-is for now to get the fix in.
>>
>>
http://codereview.chromium.**org/10411059/<http://codereview.chromium.org/104...
>>
>