1771608 - Eliminate mozilla/reject-osfile eslint warning in devTools code

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This means migrating from OS.Path to PathUtils and OS.File to IOUtils.

There are some small performance gains to be made by doing this but nothing major.

See https://firefox-source-docs.mozilla.org/dom/ioutils_migration.html for details.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1771608 - Eliminate mozilla/reject-osfile eslint warning in devTools code r=#devtools-reviewers

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Nothing to see here ;)

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Bug 1771608 - Eliminate mozilla/reject-osfile eslint warning in devTools code r=#devtools-reviewers

Removed Some Osfile.jsm and ChromeUtils Dependencies

-  const { OS } = require("resource://gre/modules/osfile.jsm");

And / Or

-  const ChromeUtils = require("ChromeUtils");

• devtools/client/memory/actions/io.js
• devtools/client/memory/utils.js
• devtools/client/netmonitor/src/har/har-menu-utils.js
• devtools/client/performance-new/utils.js
• devtools/client/responsive/test/browser/browser_screenshot_button.js
• devtools/client/shared/remote-debugging/adb/adb-binary.js
• devtools/client/shared/screenshot.js
• devtools/client/styleeditor/StyleEditorUI.jsm
• devtools/client/styleeditor/StyleSheetEditor.jsm
• devtools/client/webconsole/components/Input/JSTerm.js
• devtools/client/webconsole/test/browser/stub-generator-helpers.js
• devtools/server/actors/heap-snapshot-file.js
• devtools/server/actors/storage.js
• devtools/server/tests/xpcshell/test_MemoryActor_saveHeapSnapshot_01.js
• devtools/server/tests/xpcshell/test_MemoryActor_saveHeapSnapshot_02.js
• devtools/server/tests/xpcshell/test_MemoryActor_saveHeapSnapshot_03.js
• devtools/shared/DevToolsUtils.js
• devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js
• devtools/shared/tests/xpcshell/test_fetch-file.js
• testing/talos/talos/tests/devtools/addon/content/tests/toolbox/screenshot.js

IOUtils.read()

-  OS.File.read(path);
+  IOUtils.read(path);

• devtools/client/aboutdebugging/test/browser/helper-real-usb.js
• devtools/client/netmonitor/src/har/har-menu-utils.js
• devtools/client/shared/remote-debugging/adb/adb-binary.js
• devtools/client/styleeditor/StyleSheetEditor.jsm
• devtools/client/webconsole/components/Input/JSTerm.js
• devtools/client/webconsole/test/browser/browser_jsterm_file_load_save_keyboard_shortcut.js
• devtools/client/webconsole/test/browser/browser_webconsole_context_menu_export_console_output.js
• devtools/shared/DevToolsUtils.js

IOUtils.write()

-  OS.File.writeAtomic(filePath, fileContent);
+  IOUtils.write(filePath, fileContent);

• devtools/client/webconsole/test/browser/stub-generator-helpers.js
• devtools/shared/DevToolsUtils.js
• devtools/shared/tests/xpcshell/test_fetch-file.js

PathUtils.split()

-  OS.Path.split(path);
+  PathUtils.split(path);

• devtools/client/performance-new/utils.js
• devtools/client/styleeditor/StyleSheetEditor.jsm

PathUtils.join()

-  OS.Path.join(path, filename);
+  PathUtils.join(path, filename);

NOTE: If filename is an absolute path then OS.Path will ignore path and use filename but PathUtils will try to concatenate both paths. If filename can be an absolute path we need to use PathUtils.isAbsolute() and PathUtils.joinRelative() to make our desired behaviour explicit.

• devtools/client/debugger/test/mochitest/browser_dbg-chrome-create.js
• devtools/client/performance-new/utils.js
• devtools/client/shared/remote-debugging/adb/adb-binary.js
• devtools/client/styleeditor/StyleSheetEditor.jsm
• devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js

PathUtils.isAbsolute() and PathUtils.joinRelative()

-  filename = OS.Path.join(path, filename);
+  filename = PathUtils.isAbsolute(filename)
+    ? filename
+    : PathUtils.joinRelative(path, filename);

• devtools/client/shared/screenshot.js

IOUtils.remove()

-  OS.File.remove(filePath);
+  IOUtils.remove(filePath);

• devtools/client/framework/test/browser_toolbox_screenshot_tool.js
• devtools/client/responsive/test/browser/browser_screenshot_button.js
• devtools/client/responsive/test/browser/browser_screenshot_button_warning.js
• devtools/client/shared/test/shared-head.js
• devtools/client/webconsole/test/browser/browser_console_screenshot.js
• devtools/client/webconsole/test/browser/browser_jsterm_screenshot_command_file.js
• devtools/client/webconsole/test/browser/browser_jsterm_screenshot_command_fixed_header.js
• devtools/client/webconsole/test/browser/browser_jsterm_screenshot_command_selector.js
• testing/talos/talos/tests/devtools/addon/content/tests/toolbox/screenshot.js

PathUtils.toFileURI()

-  OS.Path.toFileURI(filePath);
+  PathUtils.toFileURI(filePath);

• devtools/client/framework/test/browser_toolbox_screenshot_tool.js
• devtools/client/responsive/test/browser/browser_screenshot_button.js
• devtools/client/shared/test/shared-head.js
• devtools/client/webconsole/test/browser/browser_console_screenshot.js
• devtools/client/webconsole/test/browser/browser_jsterm_screenshot_command_file.js
• devtools/client/webconsole/test/browser/browser_jsterm_screenshot_command_fixed_header.js
• devtools/client/webconsole/test/browser/browser_jsterm_screenshot_command_selector.js

PathUtils.filename()

-  OS.Path.basename(path),
+  PathUtils.filename(path),

• devtools/client/memory/actions/io.js
• devtools/client/memory/utils.js
• devtools/client/styleeditor/StyleEditorUI.jsm
• devtools/client/styleeditor/StyleSheetEditor.jsm

IOUtils.copy()

-  OS.File.copy(path, dest);
+  IOUtils.copy(path, dest);

• devtools/client/memory/actions/io.js

IOUtils.stat()

-  OS.File.stat(filePath);
+  IOUtils.stat(filePath);

The objects that these stat() versions return differ from one another. This hasn't made much difference to the codebase but our changed usage is included here for completeness:

-      this._fileModDate = info.lastModificationDate.getTime();
+      this._fileModDate = info.lastModified;

• devtools/client/memory/test/browser/browser_memory_transferHeapSnapshot_e10s_01.js
• devtools/client/memory/test/xpcshell/head.js
• devtools/client/memory/test/xpcshell/test_action-export-snapshot.js
• devtools/client/styleeditor/StyleSheetEditor.jsm
• devtools/server/actors/heap-snapshot-file.js
• devtools/server/tests/xpcshell/test_MemoryActor_saveHeapSnapshot_01.js
• devtools/server/tests/xpcshell/test_MemoryActor_saveHeapSnapshot_02.js
• devtools/server/tests/xpcshell/test_MemoryActor_saveHeapSnapshot_03.js
• devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js

IOUtils.setPermissions

-  OS.File.setPermissions(filePath, { unixMode: 0o744 });
+  IOUtils.setPermissions(filePath, 0o744);

• devtools/client/shared/remote-debugging/adb/adb-binary.js

IOUtils.makeDirectory

-  OS.File.makeDir(path);
+  IOUtils.makeDirectory(path);

• devtools/client/shared/remote-debugging/adb/adb-binary.js

IOUtils.exists

-  OS.File.exists(path);
+  IOUtils.exists(path);

• devtools/client/shared/remote-debugging/adb/adb-binary.js
• devtools/client/shared/screenshot.js
• devtools/client/webconsole/test/browser/browser_jsterm_file_load_save_keyboard_shortcut.js
• devtools/client/webconsole/test/browser/browser_webconsole_context_menu_export_console_output.js

PathUtils.profileDir, PathUtils.localProfileDir and PathUtils.tempDir

-    const profileDir = OS.Constants.Path.profileDir;
+    const profileDir = PathUtils.profileDir;

We can reduce reliance on Osfile.jsm in another bug bug this is a small step in that direction.

• devtools/client/shared/remote-debugging/adb/adb-binary.js
• devtools/server/actors/storage.js
• devtools/shared/heapsnapshot/HeapSnapshotFileUtils.js

IOUtils.getChildren(storagePath)

IOUtils does not have a direct equivalent of OS.File.DirectoryIterator() so we need to iterate more explicitely using IOUtils.getChildren().

- async findStorageTypePaths(storagePath) {
-   const iterator = new OS.File.DirectoryIterator(storagePath);
-   const typePaths = [];
-
-   await iterator.forEach(entry => {
-     if (entry.isDir) {
-       typePaths.push(entry.path);
-     }
-   });
-
-   iterator.close();
-   return typePaths;
- }

+ async findStorageTypePaths(storagePath) {
+   const children = await IOUtils.getChildren(storagePath);
+   const typePaths = [];
+
+   for (const path of children) {
+     const exists = await IOUtils.exists(path);
+     if (!exists) {
+       continue;
+     }
+
+     const stats = await IOUtils.stat(path);
+     if (stats.type === "directory") {
+       typePaths.push(path);
+     }
+   }
+
+   return typePaths;
+ }

• devtools/server/actors/storage.js

Misc

Made IOUtils and PathUtils available to DevTools modules.

   HeapSnapshot,
+  IOUtils,
   L10nRegistry,
   Localization,
   NamedNodeMap,
   NodeFilter,
+  PathUtils,
   StructuredCloneHolder,
   TelemetryStopwatch,

• devtools/shared/loader/builtin-modules.js

Comment 4

[Mathew Hodson]

Should block bug 1737308

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a471794382e
Eliminate mozilla/reject-osfile eslint warning in devTools code r=devtools-reviewers,perftest-reviewers,jdescottes,sparky

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/4a471794382e