Bug 91070 – Add window resize benchmark

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Hajime Morrita]

I don't think resizing window is the most important part of the interactivity.
But it's a great way to stress layout code.

[Hajime Morrita]

Created attachment 152148 [details]
Patch

[Hajime Morrita]

Created attachment 152735 [details]
Patch

[Ryosuke Niwa]

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.

[Hajime Morrita]

Created attachment 152906 [details]
Patch

[Hajime Morrita]

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.

[Ryosuke Niwa]

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>");

[Hajime Morrita]

Created attachment 152968 [details]
Patch for landing

[Hajime Morrita]

(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.

[WebKit Review Bot]

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

[Hajime Morrita]

Created attachment 153149 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 153149 [details]
Patch for landing

Clearing flags on attachment: 153149

Committed r123063: <http://trac.webkit.org/changeset/123063>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.