Bug 66984 - Enum-ify ResourceLoaderOptions/ThreadableLoaderOptions
Summary: Enum-ify ResourceLoaderOptions/ThreadableLoaderOptions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nate Chapin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-25 14:46 PDT by Nate Chapin
Modified: 2011-08-26 10:41 PDT (History)
7 users (show)

See Also:


Attachments
patch (33.75 KB, patch)
2011-08-25 14:51 PDT, Nate Chapin
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Update Notification.cpp too (34.45 KB, patch)
2011-08-25 15:26 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff
Patch for landing (36.82 KB, patch)
2011-08-26 09:32 PDT, Nate Chapin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.