[webkit-reviews] review granted: [Bug 51354] Don't block rendering and script execution on deferred stylesheets : [Attachment 77101] Updated patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Antti Koivisto
<koivisto at iki.fi>'s request for review:
Bug 51354: Don't block rendering and script execution on deferred stylesheets
https://bugs.webkit.org/show_bug.cgi?id=51354

Attachment 77101: Updated patch
https://bugs.webkit.org/attachment.cgi?id=77101&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=77101&action=review

Please do update the comment in Document.h that says "Track the number of
currently loading top-level stylesheets."

> WebCore/html/HTMLLinkElement.cpp:235
> -	       if (m_loading)
> -		   document()->removePendingSheet();
> +	       removePendingSheet();

HTMLLinkElement::removePendingSheet() doesn't check m_loading - is it OK to
remove the check?

> WebCore/html/HTMLLinkElement.cpp:467
> +    if (type == None)
> +	   return;

Can ASSERT_NOT_REACHED() be added here?

> WebCore/html/HTMLLinkElement.h:139
> +    PendingSheetType m_pendingSheetType;

I think that the new code would read better if this was a boolean variable -
"type" doesn't really mean anything. Looks like type None sheets shouldn't ever
be pending, so switching to a boolean would also make understanding this code
easier.