[Webkit-unassigned] [Bug 36201] hashchange event should be dispatched asynchronously

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=36201


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #51114|review?                     |review+
               Flag|                            |




--- Comment #5 from Darin Adler <darin at apple.com>  2010-03-18 17:58:10 PST ---
(From update of attachment 51114)
>  void Document::enqueueStorageEvent(PassRefPtr<Event> storageEvent)

It seems that you could rename this function to be non-storage-event-specific
as part of this "no behavior change" step. Is there a reason you didn't?

> +    void schedulePageshowEvent(bool persisted);
> +    void scheduleHashchangeEvent(const String& oldURL, const String& newURL);

> +    void schedulePopstateEvent(PassRefPtr<SerializedScriptValue> stateObject);

It's a little strange to use schedule for all these new functions, but enqueue
for the old function.

> -    m_document->dispatchWindowEvent(PageTransitionEvent::create(eventNames().pageshowEvent, true), m_document);
> +    m_document->schedulePageshowEvent(true);

Boolean arguments where we pass constnat valuesstink. And what in the world is
"persisted"?

> +    KURL oldURL;
>      bool hashChange = equalIgnoringFragmentIdentifier(url, m_URL) && url.fragmentIdentifier() != m_URL.fragmentIdentifier();
> +    if (hashChange)
> +        oldURL = m_URL;

If all we need for the old URL is a string, it's more efficient to save it in a
String local variable instead of KURL. I don't think we should bother with the
"if" statement here -- it's OK to unconditionally save the old URL since it
will just be a bump to a reference count.

Can you end up getting a lot of hashchange events all in a row? Is it OK to get
a hashchange event, later, after the URL is no longer the same at all?

I’ll say r=me but there is room for improvement here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.