Last Comment Bug 707681 - Efficient JS File API - Windows back-end
: Efficient JS File API - Windows back-end
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla16
Assigned To: David Rajchenbach-Teller [:Yoric] (please use "needinfo")
:
Mentors:
Depends on: 707679 747872 790649
Blocks: OS.File 707676 747876
  Show dependency treegraph
 
Reported: 2011-12-05 07:39 PST by David Rajchenbach-Teller [:Yoric] (please use "needinfo")
Modified: 2012-10-02 11:45 PDT (History)
5 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Prev 1. Windows back-end. (18.09 KB, patch)
2011-12-15 08:44 PST, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Windows synchronous back-end (12.88 KB, patch)
2012-04-23 03:32 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
dougt: review+
Details | Diff | Splinter Review
Windows synchronous back-end (13.74 KB, patch)
2012-04-28 05:15 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Windows synchronous back-end (10.63 KB, patch)
2012-05-07 00:37 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
taras.mozilla: review+
Details | Diff | Splinter Review
Windows synchronous back-end (10.59 KB, patch)
2012-05-11 08:27 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Windows synchronous back-end (10.40 KB, patch)
2012-05-11 08:32 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
dougt: review+
Details | Diff | Splinter Review
Windows synchronous back-end (10.46 KB, patch)
2012-05-21 03:14 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Windows synchronous back-end (9.70 KB, patch)
2012-05-21 03:18 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Companion testsuite (9.95 KB, patch)
2012-05-21 03:18 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Windows synchronous back-end (9.70 KB, patch)
2012-05-23 08:04 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Companion testsuite (10.05 KB, patch)
2012-05-23 08:04 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Companion testsuite (10.05 KB, patch)
2012-05-29 01:17 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Windows synchronous back-end (9.70 KB, patch)
2012-05-29 01:17 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review
Companion testsuite (10.07 KB, patch)
2012-06-07 08:15 PDT, David Rajchenbach-Teller [:Yoric] (please use "needinfo")
no flags Details | Diff | Splinter Review

Description David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2011-12-05 07:39:04 PST
Implement the JS File API Windows backend.

A few considerations:
- by opposition to Unix, the notion of opening a directory is somewhat unclear;
- this back-end needs to handle Windows-style sharing attributes;
- for the moment, we do not handle Windows-style permissions;
- by opposition to Unix, information on a file is spread through a number of system calls;
- we need to determine which version of Windows we target – most recent changes to system APIs seems to date back to Windows XP.
Comment 1 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2011-12-15 08:44:29 PST
Created attachment 581987 [details] [diff] [review]
Prev 1. Windows back-end.

Attaching an early preview of the Windows back-end.
Comment 2 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-04-23 03:32:26 PDT
Created attachment 617441 [details] [diff] [review]
Windows synchronous back-end

Here is a working version of the Windows back-end. Few features are supported at the moment (in particular not the security context when opening/creating a file), but it is a start.
Comment 3 (dormant account) 2012-04-26 10:58:43 PDT
Comment on attachment 617441 [details] [diff] [review]
Windows synchronous back-end

+     let libc_candidates = ["kernel32.dll"];
+     for (let i = 0; i < libc_candidates.length; ++i) {
+       try {
+         libc = ctypes.open(libc_candidates[i]);
+         break;
+       } catch (x) {
+         LOG("Could not open kernel "+libc_candidates[i]);
+       }
+     }

Do not include pointless code. Get rid of the try/catch, loop, etc.

Can post a second patch for me to review that applies on top of this one with these fixes.

+       WinFile.CloseHandle = function(fd) {
+         fd.dispose();
+         if (ctypes.winLastError) {
+           throw new OSWin.Error(ctypes.winLastError, "close");
+         }
+       };

Please don't change semantics of low level functions. This should do fd.forget() and then call close function directly + pass proper return code
Comment 4 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-04-28 05:15:08 PDT
Created attachment 619280 [details] [diff] [review]
Windows synchronous back-end

Applied feedback.
Comment 5 (dormant account) 2012-05-02 15:03:36 PDT
Comment on attachment 619280 [details] [diff] [review]
Windows synchronous back-end

wrong patch
Comment 6 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-07 00:37:37 PDT
Created attachment 621523 [details] [diff] [review]
Windows synchronous back-end

Oops, here it is.
Comment 7 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-11 08:27:26 PDT
Created attachment 623148 [details] [diff] [review]
Windows synchronous back-end

Fixed a few issues, bound |MoveFileEx| (which I will be using) instead of |MoveFile| (which I won't).
Comment 8 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-11 08:32:24 PDT
Created attachment 623150 [details] [diff] [review]
Windows synchronous back-end
Comment 9 Doug Turner (:dougt) 2012-05-16 12:58:10 PDT
Comment on attachment 623150 [details] [diff] [review]
Windows synchronous back-end

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

fix or follow up please.

::: toolkit/components/osfile/osfile_win_back.jsm
@@ +44,5 @@
> +     exports.OS.Win.File = {};
> +
> +     let LOG = OS.Shared.LOG.bind(OS.Shared, "OS.Win.File");
> +
> +     let libc = ctypes.open("kernel32.dll");

we should throw if libc isn't assigned.
Comment 10 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-21 03:14:43 PDT
Created attachment 625596 [details] [diff] [review]
Windows synchronous back-end

Fixed, thanks.
Comment 11 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-21 03:18:28 PDT
Created attachment 625599 [details] [diff] [review]
Windows synchronous back-end
Comment 12 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-21 03:18:45 PDT
Created attachment 625600 [details] [diff] [review]
Companion testsuite
Comment 13 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-23 08:04:06 PDT
Created attachment 626444 [details] [diff] [review]
Windows synchronous back-end
Comment 14 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-23 08:04:24 PDT
Created attachment 626445 [details] [diff] [review]
Companion testsuite
Comment 15 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-29 01:17:02 PDT
Created attachment 627875 [details] [diff] [review]
Companion testsuite
Comment 16 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-05-29 01:17:28 PDT
Created attachment 627876 [details] [diff] [review]
Windows synchronous back-end
Comment 17 David Rajchenbach-Teller [:Yoric] (please use "needinfo") 2012-06-07 08:15:33 PDT
Created attachment 630987 [details] [diff] [review]
Companion testsuite

Fixed typo.

Note You need to log in before you can comment on or make changes to this bug.