[Webkit-unassigned] [Bug 43400] Add priority attribute to XMLHttpRequest

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #63779|review?                     |review-
               Flag|                            |




--- Comment #25 from Adam Barth <abarth at webkit.org>  2010-09-06 00:02:16 PST ---
(From update of attachment 63779)
View in context: https://bugs.webkit.org/attachment.cgi?id=63779&action=prettypatch

Nits below.  I don't have an opinion on whether WebKit wants this feature.

> WebCore/xml/XMLHttpRequest.cpp:176
> +#endif // ENABLE(XMLHTTPREQUEST_PRIORITY)
We don't need the comment here.

> WebCore/xml/XMLHttpRequest.cpp:406
> +void XMLHttpRequest::setPriority(const String& s, ExceptionCode& ec)
Please don't use one-letter variables names.  Perhaps s => priority ?

> WebCore/xml/XMLHttpRequest.cpp:413
> +    for (int i = 0; i < 5; ++i) {
The magic constant 5 seems quite decoupled from the size of the array.  Do we have an arraysize macro in this code?  Failing that, a global constant used in place of each instance of 5 would be preferable.

> WebCore/xml/XMLHttpRequest.cpp:418
> +    }
I'd assert the invariants of m_priority at the end of this function, just to be clear about what's going on.

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