Make cloud print dialog creation function not use browser list, instead obtain profile and parent w…

Make cloud print dialog creation function not use browser list, instead obtain profile and parent window from invocation context.

http://crbug.com/129053
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138485
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138561
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138823
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138863

[Ben Goodger (Google), 2012-05-21 21:24:41]

Hi Scott, can you take a look at this? I don't have a good way of testing the
Mac changes so can you take a careful look at this? I am running through the
trybots now to see how the various unit/browser tests respond.

The gist of this change is... all the call sites for the cloud print dialog ui
can obtain a reasonable profile/dialog parent, so we should do that instead of
using BrowserList inside the cloud print dialog creator.

[Scott Byer, 2012-05-21 21:57:58]

It all LGTM.

[Albert Bodenhamer, 2012-05-21 22:36:55]

http://codereview.chromium.org/10388214/diff/4016/chrome/browser/printing/pri...
File chrome/browser/printing/print_dialog_cloud.h (right):

http://codereview.chromium.org/10388214/diff/4016/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud.h:30: gfx::NativeWindow modal_parent,
In Windows it's possible to create a modal dialog that uses the Desktop as a
parent by specifying a NULL parent.

We're not doing that anywhere (and it wouldn't be a good thing to do anyway) but
I could imagine someone passing in a NULL modal_parent and expecting that
behavior.

Maybe mention in the comments that a NULL modal_parent will result in a modeless
dialog?

http://codereview.chromium.org/10388214/diff/4016/chrome/browser/printing/pri...
File chrome/browser/printing/print_dialog_cloud_unittest.cc (right):

http://codereview.chromium.org/10388214/diff/4016/chrome/browser/printing/pri...
chrome/browser/printing/print_dialog_cloud_unittest.cc:320:
mock_flow_handler_.get(), 100, 100, std::string(), false));
You'll need to drop the width and height and pass in a file path.

This and the accompanying change to the constructor also changes the behavior of
the test.  It used to specify modal, while this will result in non-modal.  There
were some simplifications to the code a while back that I THINK mean it's no
longer relevant.

[Ben Goodger (Google), 2012-05-22 16:11:10]

OK, I've updated this fixing the tests and adding documentation to
print_dialog_cloud.h over the correct use of |modal_parent|.

[Albert Bodenhamer, 2012-05-22 16:37:48]

LGTM

Looks like there's still a build fail on the trybots, but it looks like a simple
fix.  Get that sorted and I think we're good.

Thanks for doing this.

[commit-bot: I haz the power, 2012-05-23 02:13:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ben@chromium.org/10388214/14015

[commit-bot: I haz the power, 2012-05-23 02:13:21]

Failed to apply patch for chrome/browser/printing/print_dialog_cloud.cc:
While running patch -p0 --forward --force;
patching file chrome/browser/printing/print_dialog_cloud.cc
Hunk #1 FAILED at 21.
Hunk #2 succeeded at 481 (offset -10 lines).
Hunk #3 succeeded at 498 (offset -10 lines).
Hunk #4 succeeded at 574 (offset -10 lines).
Hunk #5 succeeded at 616 (offset -10 lines).
Hunk #6 succeeded at 636 (offset -10 lines).
Hunk #7 succeeded at 652 (offset -10 lines).
Hunk #8 succeeded at 726 (offset -10 lines).
Hunk #9 succeeded at 769 (offset -10 lines).
Hunk #10 succeeded at 801 (offset -10 lines).
1 out of 10 hunks FAILED -- saving rejects to file
chrome/browser/printing/print_dialog_cloud.cc.rej