Bug 66984

Summary: Enum-ify ResourceLoaderOptions/ThreadableLoaderOptions
Product: WebKit Reporter: Nate Chapin <japhet>
Component: WebCore Misc.Assignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, dglazkov, eric, koivisto, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
webkit-ews: commit-queue-
Update Notification.cpp too
none
Patch for landing none

Description Nate Chapin 2011-08-25 14:46:00 PDT
Follow-up to https://bugs.webkit.org/show_bug.cgi?id=63301.

We have long lists of booleans that get passed all around and are easy to get wrong.  They'd be more readable as enums.

I'm targeting sendLoadCallbacks, sniffContent, and shoulBufferData in ResourceLoaderOptions, and allowCredentials in ThreadableLoaderOptions.
Comment 1 Nate Chapin 2011-08-25 14:51:50 PDT
Created attachment 105252 [details]
patch
Comment 2 Early Warning System Bot 2011-08-25 15:23:27 PDT
Comment on attachment 105252 [details]
patch

Attachment 105252 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9508591
Comment 3 Nate Chapin 2011-08-25 15:26:53 PDT
Created attachment 105260 [details]
Update Notification.cpp too
Comment 4 David Levin 2011-08-25 16:59:06 PDT
Comment on attachment 105260 [details]
Update Notification.cpp too

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

> Source/WebCore/loader/cache/CachedResourceRequest.cpp:128
> +                                                                                        ResourceLoaderOptions(sendResourceLoadCallbacks ? SendCallbacks : DoNotSendCallbacks, SniffContent, BufferData));

You may want to consider making this function take the enum instead of a bool.
Comment 5 WebKit Review Bot 2011-08-25 17:45:12 PDT
Comment on attachment 105260 [details]
Update Notification.cpp too

Attachment 105260 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9507684
Comment 6 Eric Seidel (no email) 2011-08-25 21:18:12 PDT
Comment on attachment 105260 [details]
Update Notification.cpp too

Attachment 105260 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/9495842
Comment 7 Nate Chapin 2011-08-26 09:15:11 PDT
(In reply to comment #4)
> (From update of attachment 105260 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=105260&action=review
> 
> > Source/WebCore/loader/cache/CachedResourceRequest.cpp:128
> > +                                                                                        ResourceLoaderOptions(sendResourceLoadCallbacks ? SendCallbacks : DoNotSendCallbacks, SniffContent, BufferData));
> 
> You may want to consider making this function take the enum instead of a bool.

I'm going to do that as a part of my next attempt at https://bugs.webkit.org/show_bug.cgi?id=66018.
Comment 8 Nate Chapin 2011-08-26 09:32:57 PDT
Created attachment 105363 [details]
Patch for landing
Comment 9 WebKit Review Bot 2011-08-26 10:41:41 PDT
Comment on attachment 105363 [details]
Patch for landing

Clearing flags on attachment: 105363

Committed r93886: <http://trac.webkit.org/changeset/93886>
Comment 10 WebKit Review Bot 2011-08-26 10:41:46 PDT
All reviewed patches have been landed.  Closing bug.