Skip to content

Commit

Permalink
Bug 1776480 - Remove OS.File et al. r=Gijs,webidl,smaug
Browse files Browse the repository at this point in the history
This patch removes the vast majority of OS.File and support code. A few things remain:

- The nsIOSFileConstantsService still exists, but the path related constants
  (OS.Constants.Path.*) are no longer added to the OS object. The plan is to
  replace this with a proper service e.g. Services.osConstants or similar) in
  bug 1786885.
- There is still support for OS.File errors in ErrorSanitizer, which will be
  removed in bug 1775167.
- The OS.File to IOUtils migration guide will be rewritten as general IOUtils
  documentation in bug 1830097.
- dom/base/Document.cpp has a workaround for not loading osfile.jsm at startup,
  which may want to be reconsidered in bug 1830100.

So long, and thanks for all the I/O.

Differential Revision: https://phabricator.services.mozilla.com/D176543
  • Loading branch information
brennie committed May 12, 2023
1 parent 0d40c21 commit 3fd4e74
Show file tree
Hide file tree
Showing 104 changed files with 3 additions and 17,821 deletions.
1 change: 0 additions & 1 deletion .eslintrc-test-paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ const extraChromeTestPaths = [
"layout/svg/tests/",
"layout/xul/test/",
"toolkit/components/aboutmemory/tests/",
"toolkit/components/osfile/tests/mochi/",
"toolkit/components/printing/tests/",
"toolkit/components/url-classifier/tests/mochitest/",
"toolkit/components/viewsource/test/",
Expand Down
8 changes: 0 additions & 8 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ module.exports = {
"toolkit/components/narrate/Narrator.jsm",
"toolkit/components/nimbus/**",
"toolkit/components/normandy/**",
"toolkit/components/osfile/**",
"toolkit/components/passwordmgr/**",
"toolkit/components/pdfjs/**",
"toolkit/components/pictureinpicture/**",
Expand Down Expand Up @@ -613,13 +612,6 @@ module.exports = {
"devtools/**",
],
},
{
// Turn off the osfile rule for osfile.
files: ["toolkit/components/osfile/**"],
rules: {
"mozilla/reject-osfile": "off",
},
},
{
// Exempt files with these paths since they have to use http for full coverage
files: httpTestingPaths.map(path => `${path}**`),
Expand Down
1 change: 0 additions & 1 deletion browser/base/content/test/performance/browser_startup.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ const startupPhases = {
modules: new Set([
"resource://gre/modules/AsyncPrefs.sys.mjs",
"resource://gre/modules/LoginManagerContextMenu.sys.mjs",
"resource://gre/modules/osfile.jsm",
"resource://pdf.js/PdfStreamConverter.sys.mjs",
]),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,6 @@ var gExceptionPaths = [

// Localization file added programatically in featureCallout.jsm
"resource://app/localization/en-US/browser/featureCallout.ftl",

// Will be removed in bug 1737308
"resource://gre/modules/lz4.js",
"resource://gre/modules/lz4_internal.js",
"resource://gre/modules/osfile.jsm",
"resource://gre/modules/osfile/",
];

// These are not part of the omni.ja file, so we find them only when running
Expand Down
1 change: 0 additions & 1 deletion docs/code-quality/lint/linters/eslint-plugin-mozilla.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ The plugin implements the following rules:
eslint-plugin-mozilla/reject-lazy-imports-into-globals
eslint-plugin-mozilla/reject-mixing-eager-and-lazy
eslint-plugin-mozilla/reject-multiple-getters-calls
eslint-plugin-mozilla/reject-osfile
eslint-plugin-mozilla/reject-relative-requires
eslint-plugin-mozilla/reject-requires-await
eslint-plugin-mozilla/reject-scriptableunicodeconverter
Expand Down

This file was deleted.

85 changes: 0 additions & 85 deletions dom/system/OSFileConstants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -835,97 +835,12 @@ bool OSFileConstantsService::DefineOSFileConstants(
return false;
}

nsCOMPtr<nsIXULRuntime> runtime =
do_GetService(XULRUNTIME_SERVICE_CONTRACTID);
if (runtime) {
nsAutoCString os;
DebugOnly<nsresult> rv = runtime->GetOS(os);
MOZ_ASSERT(NS_SUCCEEDED(rv));

JSString* strVersion = JS_NewStringCopyZ(aCx, os.get());
if (!strVersion) {
return false;
}

JS::Rooted<JS::Value> valVersion(aCx, JS::StringValue(strVersion));
if (!JS_SetProperty(aCx, objSys, "Name", valVersion)) {
return false;
}
}

#if defined(DEBUG)
JS::Rooted<JS::Value> valDebug(aCx, JS::TrueValue());
if (!JS_SetProperty(aCx, objSys, "DEBUG", valDebug)) {
return false;
}
#endif

#if defined(HAVE_64BIT_BUILD)
JS::Rooted<JS::Value> valBits(aCx, JS::Int32Value(64));
#else
JS::Rooted<JS::Value> valBits(aCx, JS::Int32Value(32));
#endif // defined (HAVE_64BIT_BUILD)
if (!JS_SetProperty(aCx, objSys, "bits", valBits)) {
return false;
}

if (!JS_DefineProperty(
aCx, objSys, "umask", mUserUmask,
JSPROP_ENUMERATE | JSPROP_READONLY | JSPROP_PERMANENT)) {
return false;
}

// Build OS.Constants.Path

JS::Rooted<JSObject*> objPath(aCx);
if (!(objPath = GetOrCreateObjectProperty(aCx, objConstants, "Path"))) {
return false;
}

// Locate libxul
// Note that we don't actually provide the full path, only the name of the
// library, which is sufficient to link to the library using js-ctypes.

#if defined(XP_MACOSX)
// Under MacOS X, for some reason, libxul is called simply "XUL",
// and we need to provide the full path.
nsAutoString libxul;
libxul.Append(mPaths->libDir);
libxul.AppendLiteral("/XUL");
#else
// On other platforms, libxul is a library "xul" with regular
// library prefix/suffix.
nsAutoString libxul;
libxul.AppendLiteral(MOZ_DLL_PREFIX);
libxul.AppendLiteral("xul");
libxul.AppendLiteral(MOZ_DLL_SUFFIX);
#endif // defined(XP_MACOSX)

if (!SetStringProperty(aCx, objPath, "libxul", libxul)) {
return false;
}

if (!SetStringProperty(aCx, objPath, "libDir", mPaths->libDir)) {
return false;
}

if (!SetStringProperty(aCx, objPath, "tmpDir", mPaths->tmpDir)) {
return false;
}

// Configure profileDir only if it is available at this stage
if (!mPaths->profileDir.IsVoid() &&
!SetStringProperty(aCx, objPath, "profileDir", mPaths->profileDir)) {
return false;
}

// Configure localProfileDir only if it is available at this stage
if (!mPaths->localProfileDir.IsVoid() &&
!SetStringProperty(aCx, objPath, "localProfileDir",
mPaths->localProfileDir)) {
return false;
}

return true;
}

Expand Down
3 changes: 0 additions & 3 deletions dom/system/moz.build
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
with Files("**"):
BUG_COMPONENT = ("Core", "DOM: Core & HTML")

with Files("*OSFile*"):
BUG_COMPONENT = ("Toolkit", "OS.File")

with Files("*ocationProvider*"):
BUG_COMPONENT = ("Core", "DOM: Geolocation")

Expand Down
4 changes: 2 additions & 2 deletions dom/system/tests/chrome.ini
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[DEFAULT]
support-files =
worker_constants.js

[test_constants.xhtml]
support-files =
worker_constants.js
[test_pathutils.html]
[test_pathutils_worker.xhtml]
support-files =
Expand Down
30 changes: 0 additions & 30 deletions dom/system/tests/ioutils/test_ioutils_read_write.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@
const { Assert } = ChromeUtils.import("resource://testing-common/Assert.jsm");
const { ObjectUtils } = ChromeUtils.import("resource://gre/modules/ObjectUtils.jsm");

// This is presently only used to test compatability between OS.File and
// IOUtils when it comes to writing compressed files. The import and the
// test `test_lz4_osfile_compat` can be removed with OS.File is removed.
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");

add_task(async function test_read_failure() {
const doesNotExist = PathUtils.join(PathUtils.tempDir, "does_not_exist.tmp");
await Assert.rejects(
Expand Down Expand Up @@ -340,31 +335,6 @@
await cleanup(tmpFileName);
});

add_task(async function test_lz4_osfile_compat() {
const osfileTmpFile = PathUtils.join(PathUtils.tempDir, "test_ioutils_lz4_compat_osfile.tmp");
const ioutilsTmpFile = PathUtils.join(PathUtils.tempDir, "test_ioutils_lz4_compat_ioutils.tmp");

info("Test OS.File and IOUtils write the same file with LZ4 compression enabled")
const repeatedBytes = Uint8Array.of(...new Array(50).fill(1));
let expectedBytes = 23;
let ioutilsBytes = await IOUtils.write(ioutilsTmpFile, repeatedBytes, { compress: true });
let osfileBytes = await OS.File.writeAtomic(osfileTmpFile, repeatedBytes, { compression: "lz4" });
is(ioutilsBytes, expectedBytes, "IOUtils writes the expected number of bytes for compression");
is(osfileBytes, ioutilsBytes, "OS.File and IOUtils write the same number of bytes for LZ4 compression");

info("Test OS.File can read a file compressed by IOUtils");
const osfileReadBytes = await OS.File.read(ioutilsTmpFile, { compression: "lz4" });
ok(osfileReadBytes.every(byte => byte === 1), "OS.File can read a file compressed by IOUtils");
is(osfileReadBytes.length, 50, "OS.File reads the right number of bytes from a file compressed by IOUtils")

info("Test IOUtils can read a file compressed by OS.File");
const ioutilsReadBytes = await IOUtils.read(osfileTmpFile, { decompress: true });
ok(ioutilsReadBytes.every(byte => byte === 1), "IOUtils can read a file compressed by OS.File");
is(ioutilsReadBytes.length, 50, "IOUtils reads the right number of bytes from a file compressed by OS.File")

await cleanup(osfileTmpFile, ioutilsTmpFile);
});

add_task(async function test_lz4_bad_call() {
const tmpFileName = PathUtils.join(PathUtils.tempDir, "test_ioutils_lz4_bad_call.tmp");

Expand Down
28 changes: 0 additions & 28 deletions dom/system/tests/ioutils/test_ioutils_read_write_utf8.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@
const { Assert } = ChromeUtils.import("resource://testing-common/Assert.jsm");
const { ObjectUtils } = ChromeUtils.import("resource://gre/modules/ObjectUtils.jsm");

// This is presently only used to test compatability between OS.File and
// IOUtils when it comes to writing compressed files. The import and the
// test `test_lz4_osfile_compat` can be removed with OS.File is removed.
const { OS } = ChromeUtils.import("resource://gre/modules/osfile.jsm");

// This is an impossible sequence of bytes in an UTF-8 encoded file.
// See section 3.5.3 of this text:
// https://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-test.txt
Expand Down Expand Up @@ -290,29 +285,6 @@
await cleanup(tmpFileName);
});

add_task(async function test_utf8_lz4_osfile_compat() {
const osfileTmpFile = PathUtils.join(PathUtils.tempDir, "test_ioutils_utf8_lz4_compat_osfile.tmp");
const ioutilsTmpFile = PathUtils.join(PathUtils.tempDir, "test_ioutils_utf8_lz4_compat_ioutils.tmp");

info("Test OS.File and IOUtils write the same UTF-8 file with LZ4 compression enabled")
const emoji = "☕️ ⚧️ 😀 🖖🏿 🤠 🏳️‍🌈 🥠 🏴‍☠️ 🪐";
let expectedBytes = 83;
let ioutilsBytes = await IOUtils.writeUTF8(ioutilsTmpFile, emoji, { compress: true });
let osfileBytes = await OS.File.writeAtomic(osfileTmpFile, emoji, { compression: "lz4" });
is(ioutilsBytes, expectedBytes, "IOUtils writes the expected number of bytes for compression");
is(osfileBytes, ioutilsBytes, "OS.File and IOUtils write the same number of bytes for LZ4 compression");

info("Test OS.File can read an UTF-8 file compressed by IOUtils");
const osfileReadStr = await OS.File.read(ioutilsTmpFile, { compression: "lz4", encoding: "utf-8" });
is(osfileReadStr, emoji, "OS.File can read an UTF-8 file compressed by IOUtils")

info("Test IOUtils can read an UTF-8 file compressed by OS.File");
const ioutilsReadString = await IOUtils.readUTF8(ioutilsTmpFile, { decompress: true });
is(ioutilsReadString, emoji, "IOUtils can read an UTF-8 file compressed by OS.File");

await cleanup(osfileTmpFile, ioutilsTmpFile);
});

add_task(async function test_utf8_lz4_bad_call() {
const tmpFileName = PathUtils.join(PathUtils.tempDir, "test_ioutils_utf8_lz4_bad_call.tmp");

Expand Down
25 changes: 0 additions & 25 deletions dom/system/tests/test_constants.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,6 @@

let worker;

function test_xul() {
let lib;
isnot(null, OS.Constants.Path.libxul, "libxulpath is defined");
try {
lib = ctypes.open(OS.Constants.Path.libxul);
lib.declare("DumpJSStack", ctypes.default_abi, ctypes.void_t);
} catch (x) {
ok(false, "Could not open libxul " + x);
}
if (lib) {
lib.close();
}
ok(true, "test_xul: opened libxul successfully");
}

// Test that OS.Constants.libc is defined
function test_libc() {
isnot(null, OS.Constants.libc, "OS.Constants.libc is defined");
Expand Down Expand Up @@ -58,11 +43,6 @@ function test_Win() {
}
}

// Test that OS.Constants.Sys.DEBUG is set properly on main thread
function test_debugBuildMainThread(isDebugBuild) {
is(isDebugBuild, !!OS.Constants.Sys.DEBUG, "OS.Constants.Sys.DEBUG is set properly on main thread");
}

// Test that OS.Constants.Sys.umask is set properly on main thread
function test_umaskMainThread(umask) {
is(umask, OS.Constants.Sys.umask,
Expand All @@ -79,13 +59,9 @@ function test() {
getService(Ci.nsIOSFileConstantsService).
init();
({ctypes} = ChromeUtils.import("resource://gre/modules/ctypes.jsm"));
test_xul();
test_libc();
test_Win();

let isDebugBuild = Cc["@mozilla.org/xpcom/debug;1"]
.getService(Ci.nsIDebug2).isDebugBuild;
test_debugBuildMainThread(isDebugBuild);

let umask = Cc["@mozilla.org/system-info;1"].
getService(Ci.nsIPropertyBag2).
Expand Down Expand Up @@ -123,7 +99,6 @@ function test() {
// pass expected values that are unavailable off-main-thread
// to the worker
worker.postMessage({
isDebugBuild: isDebugBuild,
umask: umask
});
ok(true, "test_constants.xhtml: Test in progress");
Expand Down
45 changes: 1 addition & 44 deletions dom/system/tests/worker_constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,9 @@ self.onmessage = function(msg) {
self.onmessage = function(msgInner) {
log("ignored message " + JSON.stringify(msgInner.data));
};
let { isDebugBuild, umask } = msg.data;
let { umask } = msg.data;
try {
test_name();
test_xul();
test_debugBuildWorkerThread(isDebugBuild);
test_umaskWorkerThread(umask);
test_bits();
} catch (x) {
log("Catching error: " + x);
log("Stack: " + x.stack);
Expand All @@ -45,20 +41,6 @@ function isnot(a, b, description) {
send({ kind: "isnot", a, b, description });
}

// Test that OS.Constants.Sys.Name is defined
function test_name() {
isnot(null, OS.Constants.Sys.Name, "OS.Constants.Sys.Name is defined");
}

// Test that OS.Constants.Sys.DEBUG is set properly in ChromeWorker thread
function test_debugBuildWorkerThread(isDebugBuild) {
is(
isDebugBuild,
!!OS.Constants.Sys.DEBUG,
"OS.Constants.Sys.DEBUG is set properly on worker thread"
);
}

// Test that OS.Constants.Sys.umask is set properly in ChromeWorker thread
function test_umaskWorkerThread(umask) {
is(
Expand All @@ -68,28 +50,3 @@ function test_umaskWorkerThread(umask) {
("0000" + umask.toString(8)).slice(-4)
);
}

// Test that OS.Constants.Path.libxul lets us open libxul
function test_xul() {
let lib;
isnot(null, OS.Constants.Path.libxul, "libxul is defined");
try {
lib = ctypes.open(OS.Constants.Path.libxul);
lib.declare("DumpJSStack", ctypes.default_abi, ctypes.void_t);
} catch (x) {
ok(false, "test_xul: Could not open libxul: " + x);
}
if (lib) {
lib.close();
}
ok(true, "test_xul: opened libxul successfully");
}

// Check if the value of OS.Constants.Sys.bits is 32 or 64
function test_bits() {
is(
OS.Constants.Sys.bits,
ctypes.int.ptr.size * 8,
"OS.Constants.Sys.bits is either 32 or 64"
);
}
Loading

0 comments on commit 3fd4e74

Please sign in to comment.