Closed Bug 907767 Opened 6 years ago Closed 6 years ago

Make HTMLInputElement::OpenDirectoryPicker dispatch progress events

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file, 1 obsolete file)

We need HTMLInputElement::OpenDirectoryPicker to dispatch progress events.
Attached patch patch (obsolete) — Splinter Review
Attachment #793521 - Flags: review?(bugs)
Comment on attachment 793521 [details] [diff] [review]
patch


>+#define PROGRESS_STR "progress"
>+static const uint32_t NS_PROGRESS_EVENT_INTERVAL = 50; // ms
s/NS_PROGRESS_EVENT_INTERVAL/kProgressEventInterval/

>+HTMLInputElement::StartProgressEventTimer()
>+{
>+  if (!mProgressTimer) {
>+    mProgressTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
>+  }
>+  if (mProgressTimer) {
>+    mProgressTimerIsActive = true;
>+    mProgressTimer->Cancel();
>+    mProgressTimer->InitWithCallback(this, NS_PROGRESS_EVENT_INTERVAL,
>+                                          nsITimer::TYPE_ONE_SHOT);
The last param should be aligned

>+HTMLInputElement::MaybeDispatchProgressEvent(bool aFinalProgress)
>+{
>+  if (aFinalProgress && mProgressTimerIsActive) {
>+    mProgressTimerIsActive = false;
>+    mProgressTimer->Cancel();
ProgressTimer may keep the last reference to the element, so after Cancel() the object could be dead
(well, deferred deletion of cycle collectable objects should help here, but let's not rely on that on purpose).



> class HTMLInputElement MOZ_FINAL : public nsGenericHTMLFormElementWithState,
>                                    public nsImageLoadingContent,
>                                    public nsIDOMHTMLInputElement,
>                                    public nsITextControlElement,
>                                    public nsIPhonetic,
>                                    public nsIDOMNSEditableElement,
>+                                   public nsITimerCallback,
You're missing nsITimerCallback in the QI implementation
Attachment #793521 - Flags: review?(bugs) → review-
Attached patch patchSplinter Review
Attachment #793521 - Attachment is obsolete: true
Attachment #799492 - Flags: review?(bugs)
Comment on attachment 799492 [details] [diff] [review]
patch

Just inline HandleProgressTimerCallback to HTMLInputElement::Notify.
HandleProgressTimerCallback itself isn't too useful

Could you use mozilla::Atomic<uint32_t> for mFileListProgress
Attachment #799492 - Flags: review?(bugs) → review+
https://hg.mozilla.org/mozilla-central/rev/ac51f4fe9299
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.