Closed Bug 793682 Opened 12 years ago Closed 12 years ago

[OS.File] getCurrentDirectory/setCurrentDirectory for worker threads

Categories

(Toolkit Graveyard :: OS.File, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla19

People

(Reporter: Yoric, Assigned: w1990ww1990w)

Details

(Whiteboard: [mentor=Yoric][lang=js])

Attachments

(1 file, 4 obsolete files)

Currently, in OS.File, on the main thread (osfile_{win, unix}_front.jsm), we use the |OS.File.curDir| getter/setter pair to get/set the current directory. On the worker thread (osfile_async_front.jsm), we use |OS.File.{getCurrentDirectory/setCurrentDirectory}|. For harmonization purposes, we should deprecate |OS.File.curDir| and implement get/setCurrentDirectory on the main thread.
Can I get assigned on this?
Sure, but I would like you to finish bug 793435 first.
(In reply to David Rajchenbach Teller [:Yoric] from comment #2) > Sure, but I would like you to finish bug 793435 first. Is bug 793435 finished?
Assignee: nobody → w1990ww1990w
Attached patch testing needed (obsolete) — Splinter Review
testing is needed
Attachment #671415 - Flags: review?(dteller)
Comment on attachment 671415 [details] [diff] [review] testing needed Review of attachment 671415 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, with minor fixes. Can you fix these? I will get this through the TryServer. ::: toolkit/components/osfile/osfile_unix_front.jsm @@ +857,5 @@ > set: function(path) { > throw_on_negative("curDir", > UnixFile.chdir(path) > ); > }, You should avoid duplicating code. Now that we have File.{get, set}CurrentDir, curDir getter/setter should be reimplemented to call these functions - both under Unix and Windows. ::: toolkit/components/osfile/tests/mochi/worker_test_osfile_front.js @@ +694,5 @@ > && change.getTime() <= stopMs, > "test_info: file 2 has changed between the start of the test and now - " + start + ", " + stop + ", " + change); > > // Test OS.File.stat on directory > + info = OS.File.stat(OS.File.getCurrentDirectory();); Typo.
Attachment #671415 - Flags: review?(dteller) → review+
Attachment #671415 - Attachment is obsolete: true
Attached patch new patch (obsolete) — Splinter Review
Attachment #671442 - Flags: review?(dteller)
Comment on attachment 671442 [details] [diff] [review] new patch Review of attachment 671442 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, thanks.
Attachment #671442 - Flags: review?(dteller) → review+
Attachment #671442 - Attachment is obsolete: true
Attached patch testing is need for this patch. (obsolete) — Splinter Review
Attachment #671454 - Flags: review+
This doesn't apply cleanly to inbound. Please rebase.
Keywords: checkin-needed
Here we go.
Attachment #671454 - Attachment is obsolete: true
Attachment #672200 - Flags: review+
Keywords: checkin-needed
Summary: [OS.File] getCurrentDirectory/setCurrentDirectory for the main thread → [OS.File] getCurrentDirectory/setCurrentDirectory for worker threads
This still doesn't apply cleanly. Is your copy of inbound up to date? applying 793682 patching file toolkit/components/osfile/osfile_unix_front.jsm Hunk #1 FAILED at 826 1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/osfile/osfile_unix_front.jsm.rej patching file toolkit/components/osfile/osfile_win_front.jsm Hunk #1 FAILED at 820 1 out of 1 hunks FAILED -- saving rejects to file toolkit/components/osfile/osfile_win_front.jsm.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh 793682
Keywords: checkin-needed
Why do we support setting the current directory at all? I cannot think of a use case in product code where we'd want to allow that.
(In reply to Benjamin Smedberg [:bsmedberg] from comment #13) > Why do we support setting the current directory at all? I cannot think of a > use case in product code where we'd want to allow that. I am sure that some people can think of use cases for B2G. Also, besides product code, I can also think of use cases for testing purposes.
(In reply to Ryan VanderMeulen from comment #12) > This still doesn't apply cleanly. Is your copy of inbound up to date? My bad, it seems that it wasn't.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: