Bug 215349

Summary: Clean up DragApplicationFlags after switch to OptionSet<>
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: WebCore Misc.Assignee: David Kilzer (:ddkilzer) <ddkilzer>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, cdumez, cgarcia, darin, ews-watchlist, gustavo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 215341    
Bug Blocks:    
Attachments:
Description Flags
Patch v1
none
Patch v2 none

Description David Kilzer (:ddkilzer) 2020-08-10 16:54:17 PDT
Bug 215341 fixed a WebContent crash, but there are a couple things to clean up:

1. DragApplicationFlags::DragApplicationNone can be removed.
2. DragApplicationFlags should be an enum class so DragApplicationFlags::DragApplicationIsModal becomes DragApplicationFlags::IsModal.
3. -[WebView applicationFlags:] should return an OptionSet<WebCore::DragApplicationFlags>.  (Fix call sites that use it.)
4. Source/WebKit/UIProcess/API/gtk/DropTargetGtk3.cpp should use OptionSet<WebCore::DragApplicationFlags> within the function.
Comment 1 David Kilzer (:ddkilzer) 2020-08-10 16:56:41 PDT
(In reply to David Kilzer (:ddkilzer) from comment #0)
> Bug 215341 fixed a WebContent crash, but there are a couple things to clean
> up:
> 
> 1. DragApplicationFlags::DragApplicationNone can be removed.

Chris already fixed this in Bug 215341.
Comment 2 David Kilzer (:ddkilzer) 2020-08-14 18:36:24 PDT
Created attachment 406644 [details]
Patch v1
Comment 3 EWS Watchlist 2020-08-14 18:37:18 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 David Kilzer (:ddkilzer) 2020-08-15 08:14:06 PDT
Created attachment 406667 [details]
Patch v2
Comment 5 Darin Adler 2020-08-15 12:27:22 PDT
Comment on attachment 406667 [details]
Patch v2

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

Seems like a GTK test related to the patch failed. I pushed the retry button so it would try again.

> Source/WebCore/platform/DragData.h:61
> +enum class DragApplicationFlags : uint8_t {

The enum represents a single flag. Doesn’t make sense to call it "flags". An OptionSet, of course, contains multiple flags. So this should be renamed DragApplicationFlag, singular.
Comment 6 David Kilzer (:ddkilzer) 2020-08-17 09:26:49 PDT
Comment on attachment 406667 [details]
Patch v2

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

>> Source/WebCore/platform/DragData.h:61
>> +enum class DragApplicationFlags : uint8_t {
> 
> The enum represents a single flag. Doesn’t make sense to call it "flags". An OptionSet, of course, contains multiple flags. So this should be renamed DragApplicationFlag, singular.

I'm assuming this can be done in a separate patch.
Comment 7 EWS 2020-08-17 09:33:59 PDT
Committed r265756: <https://trac.webkit.org/changeset/265756>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 406667 [details].
Comment 8 Radar WebKit Bug Importer 2020-08-17 09:34:26 PDT
<rdar://problem/67250771>