Bug 236871 – Optimize DOM storage event dispatch

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Optimize DOM storage event dispatch.

[Chris Dumez]

Created attachment 452601 [details]
Patch

[Chris Dumez]

Created attachment 452650 [details]
Patch

[Sihui Liu]

Comment on attachment 452650 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=452650&action=review

r=me

> Source/WebCore/storage/StorageEventDispatcher.cpp:55
> +        if (!frame->document() || !frame->window())
>              continue;
> -        if (sourceFrame != frame && frame->document()->securityOrigin().equal(securityOrigin.securityOrigin().ptr()))
> -            frames.append(frame);
> +        if (!frame->window()->hasEventListeners(eventNames().storageEvent))

window() also checks document(). maybe something like:
if (auto* window = frame->window(); !window || !window->hasEventListeners(eventNames().storageEvent)) continue;

> Source/WebCore/storage/StorageEventDispatcher.cpp:77
> +            if (!frame->document() || !frame->window())
> +                continue;
> +            if (!frame->window()->hasEventListeners(eventNames().storageEvent))

Ditto.

[Chris Dumez]

(In reply to Sihui Liu from comment #3)
> Comment on attachment 452650 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=452650&action=review
> 
> r=me

No r+? :)

> > Source/WebCore/storage/StorageEventDispatcher.cpp:55
> > +        if (!frame->document() || !frame->window())
> >              continue;
> > -        if (sourceFrame != frame && frame->document()->securityOrigin().equal(securityOrigin.securityOrigin().ptr()))
> > -            frames.append(frame);
> > +        if (!frame->window()->hasEventListeners(eventNames().storageEvent))
> 
> window() also checks document(). maybe something like:
> if (auto* window = frame->window(); !window ||
> !window->hasEventListeners(eventNames().storageEvent)) continue;
> 
> > Source/WebCore/storage/StorageEventDispatcher.cpp:77
> > +            if (!frame->document() || !frame->window())
> > +                continue;
> > +            if (!frame->window()->hasEventListeners(eventNames().storageEvent))
> 
> Ditto.

[Chris Dumez]

Created attachment 452669 [details]
Patch

[Chris Dumez]

(In reply to Chris Dumez from comment #4)
> (In reply to Sihui Liu from comment #3)
> > Comment on attachment 452650 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=452650&action=review
> > 
> > r=me
> 
> No r+? :)
> 
> > > Source/WebCore/storage/StorageEventDispatcher.cpp:55
> > > +        if (!frame->document() || !frame->window())
> > >              continue;
> > > -        if (sourceFrame != frame && frame->document()->securityOrigin().equal(securityOrigin.securityOrigin().ptr()))
> > > -            frames.append(frame);
> > > +        if (!frame->window()->hasEventListeners(eventNames().storageEvent))
> > 
> > window() also checks document(). maybe something like:
> > if (auto* window = frame->window(); !window ||
> > !window->hasEventListeners(eventNames().storageEvent)) continue;
> > 
> > > Source/WebCore/storage/StorageEventDispatcher.cpp:77
> > > +            if (!frame->document() || !frame->window())
> > > +                continue;
> > > +            if (!frame->window()->hasEventListeners(eventNames().storageEvent))
> > 
> > Ditto.

Thanks for reviewing, I made the suggested changes.

[Sihui Liu]

(In reply to Chris Dumez from comment #6)
> (In reply to Chris Dumez from comment #4)
> > (In reply to Sihui Liu from comment #3)
> > > Comment on attachment 452650 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=452650&action=review
> > > 
> > > r=me
> > 
> > No r+? :)

r+

> > 
> > > > Source/WebCore/storage/StorageEventDispatcher.cpp:55
> > > > +        if (!frame->document() || !frame->window())
> > > >              continue;
> > > > -        if (sourceFrame != frame && frame->document()->securityOrigin().equal(securityOrigin.securityOrigin().ptr()))
> > > > -            frames.append(frame);
> > > > +        if (!frame->window()->hasEventListeners(eventNames().storageEvent))
> > > 
> > > window() also checks document(). maybe something like:
> > > if (auto* window = frame->window(); !window ||
> > > !window->hasEventListeners(eventNames().storageEvent)) continue;
> > > 
> > > > Source/WebCore/storage/StorageEventDispatcher.cpp:77
> > > > +            if (!frame->document() || !frame->window())
> > > > +                continue;
> > > > +            if (!frame->window()->hasEventListeners(eventNames().storageEvent))
> > > 
> > > Ditto.
> 
> Thanks for reviewing, I made the suggested changes.

[EWS]

Committed r290223 (247549@main): <https://commits.webkit.org/247549@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 452669 [details].

[Radar WebKit Bug Importer]

<rdar://problem/89197481>