Make it safer.
Created attachment 347081 [details] patch
Comment on attachment 347081 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=347081&action=review Looks good! r=me > Source/WebCore/page/ActivityState.cpp:54 > + appendIf(ActivityState::IsCapturingMedia, "capturing media"); Nice cleanup! > Source/WebCore/page/FocusController.cpp:356 > + m_page.setActivityState(focused ? m_activityState | ActivityState::IsFocused : m_activityState - ActivityState::IsFocused); It's too bad we still have to use all of this bitwise math for an OptionSet. It would be much clearer to say something like "m_activityState.remove(ActivityState::IsFocused) > Source/WebCore/page/Page.cpp:1652 > + state -= { ActivityState::IsVisible, ActivityState::IsVisibleOrOccluded }; Again, these bitwise operators are hard to reason about when reading the code. Seems like extending OptionSet with helpful methods would make these kinds of statements easier to understand.
> Again, these bitwise operators are hard to reason about when reading the > code. Seems like extending OptionSet with helpful methods would make these > kinds of statements easier to understand. Yeah, at least add/remove would be good for readability.
Created attachment 347153 [details] patch
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
Created attachment 347157 [details] patch
/Volumes/Data/EWS/WebKit/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:2316:59: error: no member named 'AllFlags' in 'WebCore::ActivityState' _page->activityStateDidChange(WebCore::ActivityState::AllFlags); ~~~~~~~~~~~~~~~~~~~~~~~~^ 1 error generated.
Created attachment 347204 [details] patch
Comment on attachment 347204 [details] patch Clearing flags on attachment: 347204 Committed r234920: <https://trac.webkit.org/changeset/234920>
All reviewed patches have been landed. Closing bug.
<rdar://problem/43378163>