Bug 149468 - Mavericks: Media tests start to time out after a few days of bot uptime
Summary: Mavericks: Media tests start to time out after a few days of bot uptime
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-22 10:59 PDT by Alexey Proskuryakov
Modified: 2015-09-25 15:36 PDT (History)
5 users (show)

See Also:


Attachments
proposed fix (2.85 KB, patch)
2015-09-22 11:03 PDT, Alexey Proskuryakov
cdumez: review+
ap: commit-queue-
Details | Formatted Diff | Diff
patch for landing (2.81 KB, patch)
2015-09-22 11:34 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
better fix (17.97 KB, patch)
2015-09-24 18:01 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
better fix (17.97 KB, patch)
2015-09-24 18:07 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff
better fix, patch for landing (17.93 KB, patch)
2015-09-25 09:58 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff
better fix, patch for landing (17.96 KB, patch)
2015-09-25 11:55 PDT, Alexey Proskuryakov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-09-22 10:59:32 PDT
After importing W3C media tests, we started to get so many cached media files that it started to make tests very slow, often timing out.

This looks like a CoreMedia bug that is fixed in Yosemite. I'd like to take this opportunity to add cleanup that works on newer systems too though.
Comment 1 Alexey Proskuryakov 2015-09-22 11:03:33 PDT
Created attachment 261753 [details]
proposed fix
Comment 2 Alexey Proskuryakov 2015-09-22 11:05:33 PDT
On Yosemite WK2, the media cache goes into TMPDIR/com.apple.WebKit.WebContent.Development+WebKitTestRunner/MediaCache, which is also a global directory that should not persist between test runs.

I'll look into moving it under an ephemeral WebKitTestRunner directory separately.
Comment 3 Chris Dumez 2015-09-22 11:07:57 PDT
Comment on attachment 261753 [details]
proposed fix

r=me
Comment 4 Alexey Proskuryakov 2015-09-22 11:31:52 PDT
Comment on attachment 261753 [details]
proposed fix

Hmm, not quite right. Some Yosemite bots still put the cache into /private/tmp.
Comment 5 Alexey Proskuryakov 2015-09-22 11:34:49 PDT
Created attachment 261755 [details]
patch for landing

Made deleting /private/tmp/MediaCache unconditional.

Looks like it's used on Yosemite in DumpRenderTree, but not in WebKitTestRunner. That seems wrong, and may need separate investigation.
Comment 6 WebKit Commit Bot 2015-09-22 12:29:16 PDT
Comment on attachment 261755 [details]
patch for landing

Clearing flags on attachment: 261755

Committed r190127: <http://trac.webkit.org/changeset/190127>
Comment 7 WebKit Commit Bot 2015-09-22 12:29:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Alexey Proskuryakov 2015-09-24 17:20:02 PDT
Re-opening for a better fix.
Comment 9 Alexey Proskuryakov 2015-09-24 18:01:37 PDT
Created attachment 261908 [details]
better fix
Comment 10 WebKit Commit Bot 2015-09-24 18:03:50 PDT
Attachment 261908 [details] did not pass style-queue:


ERROR: Tools/Scripts/webkitpy/port/mac.py:117:  multiple statements on one line (semicolon)  [pep8/E702] [5]
Total errors found: 1 in 16 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Alexey Proskuryakov 2015-09-24 18:07:29 PDT
Created attachment 261909 [details]
better fix

Removed a stray semicolon.
Comment 12 Darin Adler 2015-09-25 09:23:56 PDT
Comment on attachment 261909 [details]
better fix

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

> Source/WebKit2/Shared/ios/ChildProcessIOS.mm:77
> +        String defaultUserDirectorySuffix = String([[NSBundle mainBundle] bundleIdentifier]) + "+" + parameters.clientIdentifier;

I think this would read nicer with makeString rather than the typecast, and we could use a character instead of a string for '+'.

> Source/WebKit2/Shared/mac/ChildProcessMac.mm:94
> +        auto userDirectorySuffix = parameters.extraInitializationData.find(ASCIILiteral("user-directory-suffix"));

I don’t think we have an efficient version of find that takes ASCIILiteral. In fact that might force allocation of a String and skipping the more efficient override for const char*. We could add an override for ASCIILiteral, or we could just omit ASCIILiteral here and get better efficiency.

> Source/WebKit2/Shared/mac/ChildProcessMac.mm:96
> +            sandboxParameters.setUserDirectorySuffix([(String)(userDirectorySuffix->value + ASCIILiteral("/") + String([[NSBundle mainBundle] bundleIdentifier])) fileSystemRepresentation]);

I think that using makeString we can write this without all the typecasts and ASCIILiteral stuff.

> Source/WebKit2/Shared/mac/ChildProcessMac.mm:98
> +            String defaultUserDirectorySuffix = String([[NSBundle mainBundle] bundleIdentifier]) + "+" + parameters.clientIdentifier;

I think this would read nicer with makeString rather than the typecast, and we could use a character instead of a string for '+'.
Comment 13 Alexey Proskuryakov 2015-09-25 09:58:36 PDT
Created attachment 261927 [details]
better fix, patch for landing

Thank you for the great suggestions!
Comment 14 Alexey Proskuryakov 2015-09-25 11:55:57 PDT
Created attachment 261930 [details]
better fix, patch for landing
Comment 15 WebKit Commit Bot 2015-09-25 15:36:31 PDT
Comment on attachment 261930 [details]
better fix, patch for landing

Clearing flags on attachment: 261930

Committed r190257: <http://trac.webkit.org/changeset/190257>
Comment 16 WebKit Commit Bot 2015-09-25 15:36:36 PDT
All reviewed patches have been landed.  Closing bug.