1483242 - Ensure recording filenames are unique

Status: RESOLVED FIXED

(Core Graveyard :: Web Replay, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

ContentParents use a rather dumb way to pick filenames for temporary recording files and for the recordings saved at exit via the --save-recordings.  The filename is picked based on the number of elapsed milliseconds since the process was created, which can lead to the same filename being picked for multiple different recording processes, with the end result being corrupt recording files.  This is only likely to happen when using the --save-recordings option.

The attached patch fixes this problem by selecting these filenames using a static counter to ensure uniqueness.

Comment 1

[Andrew McCreight [:mccr8]]

Comment on attachment 8999961 [details] [diff] [review]
patch

Review of attachment 8999961 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ContentParent.cpp
@@ +777,3 @@
>    nsCOMPtr<nsIFile> file;
>    return !NS_FAILED(NS_GetSpecialDirectory(NS_OS_TEMP_DIR, getter_AddRefs(file)))
> +      && !NS_FAILED(file->AppendNative(nsPrintfCString("Recording%d", ++sNumTemporaryRecordings)))

You could do "TempRecording%d" here instead to avoid collisions with the non-temp saved recordings, if that might be an issue. On OSX, the temp dir has a weird name nobody will pick, but on Linux I could see somebody picking /tmp/ specifically for non-temp recordings.

Comment 2

[Andrew McCreight [:mccr8]]

AFAICT, the current method will also have a problem if somebody is recording in multiple instances of Firefox. You could put the PID of the UI process in the file name to work around that.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/83c0cdea8a9f
Ensure recording filenames are unique, r=mccr8.

Comment 4

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/83c0cdea8a9f