Printing: Catch more error conditions and remove more cruft.

Printing: Handle more error conditions and remove more cruft.

BUG=none
TEST=none

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

[Lei Zhang, 2012-03-14 05:40:05]

https://chromiumcodereview.appspot.com/9703013/diff/4001/chrome/browser/ui/we...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://chromiumcodereview.appspot.com/9703013/diff/4001/chrome/browser/ui/we...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:126:
CHECK(args->GetString(0, &json_str));
Unless the user is messing with the print preview JS / DOM through the
inspector, we should never hit these checks.

https://chromiumcodereview.appspot.com/9703013/diff/4001/chrome/renderer/prin...
File chrome/renderer/print_web_view_helper.cc (right):

https://chromiumcodereview.appspot.com/9703013/diff/4001/chrome/renderer/prin...
chrome/renderer/print_web_view_helper.cc:1007:
DidFinishPrinting(FAIL_PRINT_INIT);
InitPrintSettingsAndPrepareFrame() already displays its own error dialog. If we
return FAIL_PRINT here, we'll display a second one in DidFinishPrinting().

[Albert Bodenhamer, 2012-03-14 16:42:19]

lgtm

[kmadhusu, 2012-03-14 18:22:20]

https://chromiumcodereview.appspot.com/9703013/diff/3002/chrome/browser/ui/we...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://chromiumcodereview.appspot.com/9703013/diff/3002/chrome/browser/ui/we...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:127:
CHECK(!json_str.empty());
'if' condition is better than CHECK because we are on the browser code and we
don't want to crash the browser.

[Lei Zhang, 2012-03-14 20:09:11]

https://chromiumcodereview.appspot.com/9703013/diff/3002/chrome/browser/ui/we...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://chromiumcodereview.appspot.com/9703013/diff/3002/chrome/browser/ui/we...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:127:
CHECK(!json_str.empty());
On 2012/03/14 18:22:20, kmadhusu wrote:
> 'if' condition is better than CHECK because we are on the browser code and we
> don't want to crash the browser. 

But we want to crash if this ever happens, so we know about it through a crash
report, or through a frustrated user. Though it should never happen, thus
there's nothing to worry about. ;)

[kmadhusu, 2012-03-14 20:28:33]

https://chromiumcodereview.appspot.com/9703013/diff/3002/chrome/browser/ui/we...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://chromiumcodereview.appspot.com/9703013/diff/3002/chrome/browser/ui/we...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:127:
CHECK(!json_str.empty());
On 2012/03/14 20:09:11, Lei Zhang wrote:
> On 2012/03/14 18:22:20, kmadhusu wrote:
> > 'if' condition is better than CHECK because we are on the browser code and
we
> > don't want to crash the browser. 
> 
> But we want to crash if this ever happens, so we know about it through a crash
> report, or through a frustrated user. Though it should never happen, thus
> there's nothing to worry about. ;)

Sorry, you are adding ~15 CHECK conditions here. If any one of the conditions is
met, it will blow up the entire browser. We should handle the error cases
gracefully and say "Print Preview Failed" message rather than crashing it.

[Lei Zhang, 2012-03-14 23:26:12]

Since we're at an impasse over how to handle this, I've excluded
print_preview_handler.cc from this CL.

[kmadhusu, 2012-03-14 23:29:45]

lgtm

[commit-bot: I haz the power, 2012-03-14 23:44:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/9703013/2009

[commit-bot: I haz the power, 2012-03-15 03:19:46]

Try job failure for 9703013-2009 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-03-15 05:58:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thestig@chromium.org/9703013/2009

[commit-bot: I haz the power, 2012-03-15 07:34:02]

Change committed as 126860