Test for cancelling PDF modal dialogs.

Test for cancelling PDF modal dialogs.

This test verifies that, when navigating to an interstitial page from
a PDF that is currently displaying a modal dialog, that the dialog is
cancelled.

This test is expected to fail without
https://codereview.chromium.org/1150843002/.

BUG=482380

[wjmaclean, 2015-05-22 21:34:21]

wjmaclean@chromium.org changed reviewers:
+ creis@chromium.org

[wjmaclean, 2015-05-22 21:36:29]

Please take a look and see if this seems sane.

If the test seems reasonable, then before landing I expect we would convert the
data url to a simpler PDF (a simple alert), or just link directly to a simpler
PDF without using a data url (though I think the data URL makes this CL a bit
simpler).

[Charlie Reis, 2015-05-22 21:56:56]

On 2015/05/22 21:36:29, wjmaclean wrote:
> Please take a look and see if this seems sane.

Sure, this seems reasonable to me.  Thanks!

> If the test seems reasonable, then before landing I expect we would convert
the
> data url to a simpler PDF (a simple alert), or just link directly to a simpler
> PDF without using a data url (though I think the data URL makes this CL a bit
> simpler).

I agree that we should use a simpler PDF if you can generate one.

https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:595: GURL pdf_with_dialog(
Since we're in chrome/ anyway, it's probably worthwhile to add the actual PDF
file to chrome/test/data/pdf/, rather than using a data URL.  That would let
people try it out manually.

https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:626:
ui_test_utils::NavigateToURL(browser(), pdf_with_dialog);
Does loading a PDF in the main frame have the same bug?  I thought we only used
webview for PDFs loaded in subframes, but I certainly could be wrong.

(Does the dialog actually stay open on line 641 without my fix?)

[Fady Samuel, 2015-05-22 21:58:32]

On 2015/05/22 21:56:56, Charlie Reis wrote:
> On 2015/05/22 21:36:29, wjmaclean wrote:
> > Please take a look and see if this seems sane.
> 
> Sure, this seems reasonable to me.  Thanks!
> 
> > If the test seems reasonable, then before landing I expect we would convert
> the
> > data url to a simpler PDF (a simple alert), or just link directly to a
simpler
> > PDF without using a data url (though I think the data URL makes this CL a
bit
> > simpler).
> 
> I agree that we should use a simpler PDF if you can generate one.
> 
>
https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
> File chrome/browser/ui/browser_browsertest.cc (right):
> 
>
https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
> chrome/browser/ui/browser_browsertest.cc:595: GURL pdf_with_dialog(
> Since we're in chrome/ anyway, it's probably worthwhile to add the actual PDF
> file to chrome/test/data/pdf/, rather than using a data URL.  That would let
> people try it out manually.
> 
>
https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
> chrome/browser/ui/browser_browsertest.cc:626:
> ui_test_utils::NavigateToURL(browser(), pdf_with_dialog);
> Does loading a PDF in the main frame have the same bug?  I thought we only
used
> webview for PDFs loaded in subframes, but I certainly could be wrong.
> 
> (Does the dialog actually stay open on line 641 without my fix?)

Yes, we use GuestView for all PDFs, including the top level PDF.

[Charlie Reis, 2015-05-22 22:36:03]

Great.  And I've confirmed that the test fails without my fix and passes with
it.  Let me know once you have a simpler PDF and we can merge the CLs.  Thanks!

[wjmaclean, 2015-05-25 17:45:43]

Here's the revised test with the separate, simpler PDF file. Let me know if you
want the PHP script I used to generate the PDF, though none of it is stream
compressed so it's easy to view what it does in a text editor. I'll let you
merge it into your CL for landing.

https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
File chrome/browser/ui/browser_browsertest.cc (right):

https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:595: GURL pdf_with_dialog(
On 2015/05/22 21:56:56, Charlie Reis wrote:
> Since we're in chrome/ anyway, it's probably worthwhile to add the actual PDF
> file to chrome/test/data/pdf/, rather than using a data URL.  That would let
> people try it out manually.

Done.

https://codereview.chromium.org/1156663004/diff/1/chrome/browser/ui/browser_b...
chrome/browser/ui/browser_browsertest.cc:626:
ui_test_utils::NavigateToURL(browser(), pdf_with_dialog);
On 2015/05/22 21:56:56, Charlie Reis wrote:
> Does loading a PDF in the main frame have the same bug?  I thought we only
used
> webview for PDFs loaded in subframes, but I certainly could be wrong.
> 
> (Does the dialog actually stay open on line 641 without my fix?)

Yes, all PDFs use a webview.

[Charlie Reis, 2015-05-26 21:29:26]

Thanks!  I've added it to https://codereview.chromium.org/1150843002/, so feel
free to close this CL.

[wjmaclean, 2015-05-27 12:28:56]

On 2015/05/26 21:29:26, Charlie Reis wrote:
> Thanks!  I've added it to https://codereview.chromium.org/1150843002/, so feel
> free to close this CL.

Closing :-)