Bug 91070

Summary: Add window resize benchmark
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: Tools / TestsAssignee: Hajime Morrita <morrita>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 91190    
Bug Blocks: 90941    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing
none
Patch for landing none

Description Hajime Morrita 2012-07-12 04:15:40 PDT
I don't think resizing window is the most important part of the interactivity.
But it's a great way to stress layout code.
Comment 1 Hajime Morrita 2012-07-12 21:05:27 PDT
Created attachment 152148 [details]
Patch
Comment 2 Hajime Morrita 2012-07-17 04:40:45 PDT
Created attachment 152735 [details]
Patch
Comment 3 Ryosuke Niwa 2012-07-17 10:51:24 PDT
Comment on attachment 152735 [details]
Patch

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

> Tools/Scripts/webkitpy/performance_tests/perftest.py:338
> +        src_text = open(src_filename).read()
> +        dst_text = re.sub("<!-- WEBKIT_INTERACTIVE_TEST_TEMPLATE -->",

Please spell out source and destination. I'd prefer calling it something like original & edited though.

> Tools/Scripts/webkitpy/performance_tests/perftest.py:349
> +        src_png = os.path.join(test_root, "Parser", "resources", "greenbox.png")
> +        dst_png = os.path.join(test_root, "Interactive", "resources", "greenbox.png")
> +        shutil.copy(src_png, dst_png)
> +        src_html = os.path.join(test_root, "Parser", "resources", "html5.html")
> +        dst_html = os.path.join(test_root, "Interactive", "resources", "html5.html")

Ditto.

> PerformanceTests/Parser/resources/html5.html:557
> +  <!-- WEBKIT_INTERACTIVE_TEST_TEMPLATE -->

Can we instead add a script element along the line of:
<script>
if (window.parent.interactiveTest)
    window.parent.interactiveTest();
</script>
That'll avoid python change, and I don't think it'll add a statistically significant impact on the test result given the size of the page.
Comment 4 Hajime Morrita 2012-07-17 20:02:38 PDT
Created attachment 152906 [details]
Patch
Comment 5 Hajime Morrita 2012-07-17 20:03:38 PDT
Ryosuke, thanks for taking look at this!
I updated the patch to get rid of the modify-and-copy approach.
It looks becoming much simpler.
Comment 6 Ryosuke Niwa 2012-07-17 22:52:39 PDT
Comment on attachment 152906 [details]
Patch

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

> PerformanceTests/Interactive/window-resize.html:4
> +window.location = "../Parser/resources/html5.html?interactiveTest=exerciseWindowResize";

Please make sure this technique works on Qt, Chromium, & Mac.

> PerformanceTests/Parser/resources/html5.html:557
> +  <script><!--

Do we really need <!-- ~ -->?

> PerformanceTests/Parser/resources/html5.html:558
> +  if (0 <= window.location.search.indexOf("interactiveTest")) {

Maybe we can make it more genetic by specifying the path instead?
e.g. html5.html?inject=Interactive/resources/driver.js
and you do:
document.write("<script src='../../" + injectedScript + "'></script>");
Comment 7 Hajime Morrita 2012-07-18 02:30:55 PDT
Created attachment 152968 [details]
Patch for landing
Comment 8 Hajime Morrita 2012-07-18 02:31:53 PDT
(In reply to comment #6)
> (From update of attachment 152906 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=152906&action=review
> 
> > PerformanceTests/Interactive/window-resize.html:4
> > +window.location = "../Parser/resources/html5.html?interactiveTest=exerciseWindowResize";
> 
> Please make sure this technique works on Qt, Chromium, & Mac.
> 
> > PerformanceTests/Parser/resources/html5.html:557
> > +  <script><!--
> 
> Do we really need <!-- ~ -->?
> 
yeah, looks like...

> > PerformanceTests/Parser/resources/html5.html:558
> > +  if (0 <= window.location.search.indexOf("interactiveTest")) {
> 
> Maybe we can make it more genetic by specifying the path instead?
> e.g. html5.html?inject=Interactive/resources/driver.js
> and you do:
> document.write("<script src='../../" + injectedScript + "'></script>");
Good idea. did it.
Comment 9 WebKit Review Bot 2012-07-18 02:33:26 PDT
Comment on attachment 152968 [details]
Patch for landing

Rejecting attachment 152968 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/PerformanceTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/13275491
Comment 10 Hajime Morrita 2012-07-18 18:03:31 PDT
Created attachment 153149 [details]
Patch for landing
Comment 11 WebKit Review Bot 2012-07-18 19:29:25 PDT
Comment on attachment 153149 [details]
Patch for landing

Clearing flags on attachment: 153149

Committed r123063: <http://trac.webkit.org/changeset/123063>
Comment 12 WebKit Review Bot 2012-07-18 19:29:30 PDT
All reviewed patches have been landed.  Closing bug.