PrintPreviewPdfGeneratedBrowserTest properly listens to messages and exits correctly.

PrintPreviewPdfGeneratedBrowserTest now properly reads from |std::cin| removing an edge case where |std::cin.eof()| is false, but |input.empty()| is true. The test also properly exits and doesn't wait for SIGKILL anymore.
BUG=388517
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285459

[ivandavid, 2014-07-23 03:33:57]

https://codereview.chromium.org/414623005/diff/1/chrome/browser/printing/prin...
File chrome/browser/printing/print_preview_pdf_generated_browsertest.cc (left):

https://codereview.chromium.org/414623005/diff/1/chrome/browser/printing/prin...
chrome/browser/printing/print_preview_pdf_generated_browsertest.cc:655:
std::getline(std::cin, input);
The error would occur after at least 1 iteration of the loop starting at 653.
The problem was that |input| would be empty,  yet |std::cin.eof()| would
evaluate to false, causing the loop to exit when it shouldn't have. To prevent
this, the loop exits when |input| is empty and just keeps clearing the state of
|std::cin| since eventually the input will be correct. |input| is guaranteed to
either be empty or have the correct input due to how the layout test framework
prints everything so this works.

[Lei Zhang, 2014-07-23 17:11:43]

lgtm

It's more descriptive if you say PrintPreviewPdfGeneratedBrowserTest instead of 
MANUAL_LayoutTestDriver. If someone glances at the CL description, they will
understand the former without much context but won't understand the latter.

Please fill in the BUG= field.

Please put what you have in the subject line into the description field.
Remember, the description field is the commit message. Right now it just says
"Test now properly ..." which has no reference to what test it's fixing.

[ivandavid, 2014-07-24 01:09:38]

I added another fix to this patch. The test exits correctly now and cleans
itself up (unless it truly crashes). It does this by listening for a message and
then breaking out of the while loop once it gets that message. I modified the
description and the title accordingly.

[Lei Zhang, 2014-07-24 01:30:33]

Can you point me to the layout test Python code that emits the QUIT command?

Your description still has the old subject line.

[Lei Zhang, 2014-07-24 21:06:52]

https://codereview.chromium.org/414623005/diff/20001/chrome/browser/printing/...
File chrome/browser/printing/print_preview_pdf_generated_browsertest.cc (right):

https://codereview.chromium.org/414623005/diff/20001/chrome/browser/printing/...
chrome/browser/printing/print_preview_pdf_generated_browsertest.cc:661: if
(input.find("QUIT") != std::string::npos)
If you do this, shouldn't it be input == "QUIT" ? Also, you would want to update
the comment block above at line 647.

[ivandavid, 2014-07-24 21:39:38]

https://codereview.chromium.org/414623005/diff/20001/chrome/browser/printing/...
File chrome/browser/printing/print_preview_pdf_generated_browsertest.cc (right):

https://codereview.chromium.org/414623005/diff/20001/chrome/browser/printing/...
chrome/browser/printing/print_preview_pdf_generated_browsertest.cc:661: if
(input.find("QUIT") != std::string::npos)
On 2014/07/24 21:06:52, Lei Zhang wrote:
> If you do this, shouldn't it be input == "QUIT" ? Also, you would want to
update
> the comment block above at line 647.

Done.

https://codereview.chromium.org/414623005/diff/40001/chrome/browser/printing/...
File chrome/browser/printing/print_preview_pdf_generated_browsertest.cc (right):

https://codereview.chromium.org/414623005/diff/40001/chrome/browser/printing/...
chrome/browser/printing/print_preview_pdf_generated_browsertest.cc:650:
std::getline(std::cin, input);
I flipped the getline and the clearing of |std::cin| because I felt that it made
more sense to read first, then clear, rather than clear and then read.

[Lei Zhang, 2014-07-24 21:47:30]

Still need to update the CL description. Otherwise LGTM

[ivandavid, 2014-07-24 22:30:26]

The CQ bit was checked by ivandavid@chromium.org

[commit-bot: I haz the power, 2014-07-24 22:31:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ivandavid@chromium.org/414623005/60001

[commit-bot: I haz the power, 2014-07-25 02:37:53]

Change committed as 285459