Bug 79872

Summary: perftestsrunner can call printer.write() after printer.cleanup()
Product: WebKit Reporter: Dirk Pranke <dpranke>
Component: New BugsAssignee: Dirk Pranke <dpranke>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ojan, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
don't call run() twice instead rniwa: review+

Description Dirk Pranke 2012-02-28 22:22:11 PST
perftestsrunner can call printer.write() after printer.cleanup()
Comment 1 Dirk Pranke 2012-02-28 22:26:24 PST
PerfTestsRunner.run() calls printer.cleanup(), and you should not call any methods on printer after cleanup(). In the unittests, we call run() twice in a row (in test_run_with_upload_json), and the second time attempts to write something.                                            
                                                                                
We should either not call cleanup() during run(), or reinitialize the printer during run(), or not call run() twice.

I will post a patch that does the first thing, but I'm not sure if you're supposed to be able to call run() twice in a row or not.
Comment 2 Dirk Pranke 2012-02-28 22:28:34 PST
Created attachment 129391 [details]
Patch
Comment 3 Ryosuke Niwa 2012-02-28 22:31:39 PST
Comment on attachment 129391 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=129391&action=review

> Tools/ChangeLog:13
> +        It was a fluke that this happened to work, but it is still a bug
> +        that should be fixed either by not calling cleanup() during
> +        run() or by not calling run() twice. This patch does the former.

Can we take the latter approach instead? We shouldn't be calling run() twice.
Comment 4 Dirk Pranke 2012-02-28 22:36:10 PST
Sure, I'll rework it.
Comment 5 Dirk Pranke 2012-02-28 22:41:15 PST
Created attachment 129392 [details]
don't call run() twice instead
Comment 6 Dirk Pranke 2012-02-28 22:41:45 PST
Comment on attachment 129392 [details]
don't call run() twice instead

please take another look?
Comment 7 Dirk Pranke 2012-02-29 11:48:46 PST
Committed r109235: <http://trac.webkit.org/changeset/109235>