Closed
Bug 953425
Opened 10 years ago
Closed 10 years ago
DOMFileMetadataParameters and LockedFile cleanup
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
Attachments
(4 files, 1 obsolete file)
4.02 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
5.29 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
5.45 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
4.68 KB,
patch
|
janv
:
review+
|
Details | Diff | Splinter Review |
Want to move these out of bug 860731 so I can land them before figuring out the TypeError stuff.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8351718 -
Flags: review?(Jan.Varga)
Assignee | ||
Comment 2•10 years ago
|
||
This also helps with my goal of killing XPConnect dictionary support.
Attachment #8351719 -
Flags: review?(Jan.Varga)
Assignee | ||
Comment 3•10 years ago
|
||
This is necessary to use FileRequest in IDL.
Attachment #8355083 -
Flags: review?(Jan.Varga)
Assignee | ||
Updated•10 years ago
|
Summary: DOMFileMetadataParameters cleanup → DOMFileMetadataParameters and LockedFile cleanup
Comment 4•10 years ago
|
||
Comment on attachment 8351718 [details] [diff] [review] Part a: Store bools directly in MetadataParameters Review of attachment 8351718 [details] [diff] [review]: ----------------------------------------------------------------- Looks good
Attachment #8351718 -
Flags: review?(Jan.Varga) → review+
Updated•10 years ago
|
Attachment #8351719 -
Flags: review?(Jan.Varga) → review+
Comment 5•10 years ago
|
||
Comment on attachment 8355083 [details] [diff] [review] Part c: Use DOMFileRequest in LockedFile Review of attachment 8355083 [details] [diff] [review]: ----------------------------------------------------------------- Hm, I wonder why we didn't just add DOMFileRequest::Create()
Comment 6•10 years ago
|
||
(In reply to Jan Varga [:janv] from comment #5) > Comment on attachment 8355083 [details] [diff] [review] > Part c: Use DOMFileRequest in LockedFile > > Review of attachment 8355083 [details] [diff] [review]: > ----------------------------------------------------------------- > > Hm, I wonder why we didn't just add DOMFileRequest::Create() Ms2ger ?
Flags: needinfo?(Ms2ger)
Comment 8•10 years ago
|
||
Comment on attachment 8355083 [details] [diff] [review] Part c: Use DOMFileRequest in LockedFile Review of attachment 8355083 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/file/LockedFile.cpp @@ +422,4 @@ > LockedFile::GenerateFileRequest() > { > NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); > + return FileRequest::Create(GetOwner(), this, true).downcast<DOMFileRequest>(); I'll send an additional patch to add DOMFileRequest::Create(), I don't know why we didn't add it before. ::: dom/file/LockedFile.h @@ +23,2 @@ > class FileHandle; > class FileRequest; Nit: |class FileRequest;| is now not needed
Attachment #8355083 -
Flags: review?(Jan.Varga) → review+
Comment 9•10 years ago
|
||
Attachment #8357064 -
Flags: review?(Ms2ger)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8357064 [details] [diff] [review] Part d: Add DOMFileRequest::Create() Review of attachment 8357064 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. I guess I'll land it with my patches? ::: dom/file/DOMFileRequest.cpp @@ +19,5 @@ > +// static > +already_AddRefed<DOMFileRequest> > +DOMFileRequest::Create(nsPIDOMWindow* aOwner, LockedFile* aLockedFile) > +{ > + NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); Feel free to make this MOZ_ASSERT. ::: dom/file/FileHandle.cpp @@ +148,5 @@ > return nullptr; > } > > nsRefPtr<FileRequest> request = > + FileRequest::Create(GetOwner(), lockedFile); Check if it fits in 80 columns now.
Attachment #8357064 -
Flags: review?(Ms2ger) → review+
Comment 11•10 years ago
|
||
patch for checkin, feel free to push it
Attachment #8357064 -
Attachment is obsolete: true
Attachment #8357813 -
Flags: review+
Assignee | ||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/87a106b9ec5f https://hg.mozilla.org/mozilla-central/rev/8f02481f0771 Somehow I missed half the patches here.
Flags: in-testsuite-
Assignee | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/37c564f32e26 https://hg.mozilla.org/mozilla-central/rev/4a49a222793f
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•