Reign in print throttling.

Reign in print throttling.

Don't throttle the frequency of calls to window.print() from JavaScript if the
action was user initiated (e.g., clicking on a button), if print preview is
active (which is tab-modal, so the user can easily stop any attempted DOS), or
if a flag on the command line is given (say, you want to run chromium for a
point-of-sale system).

BUG=50186
TEST=Go to a web page with a print button - should be able to click on the button and cancel the print quickly without ever seeing the dreaded warning from 50186 in the console.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=157612

[Scott Byer, 2012-08-30 17:59:09]

An alternative would be to strip out the throttling entirely, but that could
leave users vulnerable to a bad webpage DOS attack when using the system dialog.

[Jói, 2012-08-30 18:05:22]

LGTM with a nit

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
File chrome/renderer/print_web_view_helper.h (right):

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.h:336: bool user_initiated);
On a method with this name, it feels weird to have this parameter; the method
name seems to imply user_initiated=False.  Worth an additional sentence in the
comment perhaps?

[Lei Zhang, 2012-08-30 18:15:31]

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.cc:740: // If preview is enabled, then the
print dialog is tab modal, and the user
Keep in mind there's a long term goal to make scripted printing app modal for
print preview. There's a few corner cases that print preview doesn't handle
correctly for scripted printing right now.

[Scott Byer, 2012-08-30 18:22:12]

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.cc:740: // If preview is enabled, then the
print dialog is tab modal, and the user
On 2012/08/30 18:15:31, Lei Zhang wrote:
> Keep in mind there's a long term goal to make scripted printing app modal for
> print preview. There's a few corner cases that print preview doesn't handle
> correctly for scripted printing right now.

I think the key to keeping the condition would be that the originating tab could
be closed (implicitly cancelling the print request) even with an app-modal
preview - that should be possible to do. If not, we remove the condition at that
time - I think being user initiated covers most of the cases from the bug - or
reconsider stripping out the throttling entirely at that point.

[Lei Zhang, 2012-08-30 18:34:46]

Sounds good. LGTM

[Scott Byer, 2012-08-30 23:32:24]

jamesr, ping for content/renderer change.

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
File chrome/renderer/print_web_view_helper.h (right):

http://codereview.chromium.org/10890054/diff/6001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.h:336: bool user_initiated);
On 2012/08/30 18:05:22, Jói wrote:
> On a method with this name, it feels weird to have this parameter; the method
> name seems to imply user_initiated=False.  Worth an additional sentence in the
> comment perhaps?

Done.

[jamesr, 2012-08-31 23:20:27]

+cc Necker to check the user gesture / throttling logic changes.

[Scott Byer, 2012-09-19 17:51:47]

Neckar, friendly ping?

[Cris Neckar, 2012-09-19 17:59:41]

On 2012/09/19 17:51:47, Scott Byer wrote:
> Neckar, friendly ping?
Sorry about that, missed the original mail for this. 

LGTM

[jamesr, 2012-09-19 18:12:53]

Thanks for looking, Cris. owners stamp LGTM on content/renderer

[commit-bot: I haz the power, 2012-09-19 19:02:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scottbyer@chromium.org/10890054/19001

[commit-bot: I haz the power, 2012-09-19 21:36:13]

Change committed as 157612