907767 - Make HTMLInputElement::OpenDirectoryPicker dispatch progress events

Status: RESOLVED FIXED

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

Description

[Jonathan Watt [:jwatt]]

We need HTMLInputElement::OpenDirectoryPicker to dispatch progress events.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 2

[Olli Pettay [:smaug]]

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

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 4

[Olli Pettay [:smaug]]

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

Comment 5

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac51f4fe9299

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ac51f4fe9299