Closed Bug 1771608 Opened 3 years ago Closed 2 years ago

Eliminate mozilla/reject-osfile eslint warning in devTools code

Categories

(DevTools :: General, enhancement, P3)

enhancement

Tracking

(firefox103 fixed)

RESOLVED FIXED
103 Branch
Tracking Status
firefox103 --- fixed

People

(Reporter: miker, Assigned: miker)

References

Details

Attachments

(1 file, 1 obsolete file)

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.

Attachment #9278616 - Attachment description: WIP: Bug 1771608 - Eliminate mozilla/reject-osfile eslint warning in devTools code → 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
Attachment #9278616 - Attachment is obsolete: true
Severity: -- → S3
Priority: -- → P3

Should block bug 1737308

Pushed by michael@ratcliffefamily.org: https://hg.mozilla.org/integration/autoland/rev/4a471794382e Eliminate mozilla/reject-osfile eslint warning in devTools code r=devtools-reviewers,perftest-reviewers,jdescottes,sparky
Regressions: 1772442
Regressions: 1772443
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 103 Branch
No longer blocks: 1737308
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: