Closed Bug 1376981 Opened 2 years ago Closed 2 years ago

Label nsBindingManager::DoProcessAttachedQueue runnable

Categories

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

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: billm, Assigned: billm)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
This runnable seems pretty easy to label since there's a document readily available.
Attachment #8882004 - Flags: review?(bugs)
Comment on attachment 8882004 [details] [diff] [review]
patch

> nsBindingManager::PostProcessAttachedQueueEvent()
> {
>+  MOZ_ASSERT(NS_IsMainThread());
>   mProcessAttachedQueueEvent =
>     NewRunnableMethod("nsBindingManager::DoProcessAttachedQueue",
>                       this, &nsBindingManager::DoProcessAttachedQueue);
>-  nsresult rv = NS_DispatchToCurrentThread(mProcessAttachedQueueEvent);
>+  nsresult rv = mDocument->EventTargetFor(TaskCategory::Other)->Dispatch(do_AddRef(mProcessAttachedQueueEvent));
>   if (NS_SUCCEEDED(rv) && mDocument) {
So here we do null check for mDocument. Silly to first use mDocument and then null check it.



I think you could add a null check early in the method and just return then and remove null check from the if (...mDocument)
With that, r+
Attachment #8882004 - Flags: review?(bugs) → review+
Assignee: nobody → wmccloskey
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b5ca3d02cd72
Label nsBindingManager::PostProcessAttachedQueueEvent (r=smaug)
https://hg.mozilla.org/mozilla-central/rev/b5ca3d02cd72
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.