Bug 96256 - [WK2][WTR] WebKitTestRunner needs layoutTestController.setMinimumTimerInterval
Summary: [WK2][WTR] WebKitTestRunner needs layoutTestController.setMinimumTimerInterval
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mikhail Pozdnyakov
URL:
Keywords:
Depends on: 96396
Blocks:
  Show dependency tree / graph
 
Reported: 2012-09-10 03:59 PDT by Mikhail Pozdnyakov
Modified: 2012-09-11 08:49 PDT (History)
5 users (show)

See Also:

Attachments
patch (10.26 KB, patch)
2012-09-10 04:58 PDT, Mikhail Pozdnyakov 		kenneth: review+
kenneth: commit-queue-
Details | Formatted Diff | Diff
patch v2 (10.31 KB, patch)
2012-09-10 05:43 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
patch v3 (10.50 KB, patch)
2012-09-10 06:02 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
patch v4 (10.52 KB, patch)
2012-09-11 03:03 PDT, Mikhail Pozdnyakov 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Pozdnyakov 2012-09-10 03:59:47 PDT 
WTR needs WebKitTestRunner needs layoutTestController.setMinimumTimerInterval implementation so that 
fast/dom/timer-increase-min-interval-and-reset-part-1.html
fast/dom/timer-increase-min-interval-and-reset-part-2.html
fast/dom/timer-increase-min-interval-repeating.html
fast/dom/timer-increase-min-interval.html
fast/dom/timer-increase-then-decrease-min-interval-repeating.html
fast/dom/timer-increase-then-decrease-min-interval.html
can be unskipped
Comment 1 Mikhail Pozdnyakov 2012-09-10 04:58:39 PDT 
Created attachment 163091 [details]
patch
Comment 2 Chris Dumez 2012-09-10 05:10:49 PDT 
Comment on attachment 163091 [details]
patch

Looks sane.
Comment 3 Kenneth Rohde Christiansen 2012-09-10 05:16:23 PDT 
Comment on attachment 163091 [details]
patch

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

> Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:70
> +        void setMinimumTimerInterval(in double interval);

intervalMSecs ? or what is normally used...
Comment 4 Mikhail Pozdnyakov 2012-09-10 05:43:29 PDT 
Created attachment 163099 [details]
patch v2

Added a comment to .idl file describing the measure of the given interval parameter for newly added setMinimumTimerInterval().
Comment 5 Kenneth Rohde Christiansen 2012-09-10 05:49:47 PDT 
Comment on attachment 163099 [details]
patch v2

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

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:676
> +void TestRunner::setMinimumTimerInterval(double interval)

why not just use "double seconds"
Comment 6 Mikhail Pozdnyakov 2012-09-10 06:02:52 PDT 
Created attachment 163106 [details]
patch v3

Used 'seconds' name for the interval parameter in cpp files.
Comment 7 Kenneth Rohde Christiansen 2012-09-10 08:19:10 PDT 
Comment on attachment 163106 [details]
patch v3

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

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundle.cpp:234
> +void WKBundleSetMinimumTimerInterval(WKBundleRef bundleRef, WKBundlePageGroupRef pageGroupRef, double interval)

seconds

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.h:128
> +    void setMinimumTimerInterval(WebPageGroupProxy*, double);

here you have no idea what double means, so it makes sense to add double seconds... I see it is not perfect elsewhere, but this is the right thing to do

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:109
> +    void setMinimumTimerInterval(double); // Interval specified in seconds.

double seconds
Comment 8 Mikhail Pozdnyakov 2012-09-11 03:03:38 PDT 
Created attachment 163317 [details]
patch v4

Applied comments from Kenneth.
Comment 9 WebKit Review Bot 2012-09-11 04:21:31 PDT 
Comment on attachment 163317 [details]
patch v4

Clearing flags on attachment: 163317

Committed r128169: <http://trac.webkit.org/changeset/128169>
Comment 10 WebKit Review Bot 2012-09-11 04:21:35 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Csaba Osztrogonác 2012-09-11 08:49:53 PDT 
(In reply to comment #9)
> (From update of attachment 163317 [details])
> Clearing flags on attachment: 163317
> 
> Committed r128169: <http://trac.webkit.org/changeset/128169>

It caused a regression - https://bugs.webkit.org/show_bug.cgi?id=96396
Could you check it, please?