[webkit-reviews] review granted: [Bug 123379] Change WebPage, WebPageProxy, WebPageCreationParameters to use ViewState : [Attachment 215311] Fix

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 28 09:19:19 PDT 2013


Darin Adler <darin at apple.com> has granted Gavin Barraclough
<barraclough at apple.com>'s request for review:
Bug 123379: Change WebPage, WebPageProxy, WebPageCreationParameters to use
ViewState
https://bugs.webkit.org/show_bug.cgi?id=123379

Attachment 215311: Fix
https://bugs.webkit.org/attachment.cgi?id=215311&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=215311&action=review


> Source/WebKit2/Shared/ViewState.h:42
> +    static const Flags Reset = 0;

Maybe NoFlags would be a better name than Reset?

> Source/WebKit2/UIProcess/WebPageProxy.h:755
> +    void updateViewState(ViewState::Flags = ViewState::AllFlags);

The purpose of this argument is not entirely clear. I suggest naming it
something like flagsToUpdate.

>> Source/WebKit2/WebProcess/WebPage/WebPage.h:687
>> +	void setIsInWindow(bool isInWindow);
> 
> The parameter name "isInWindow" adds no information, so it should be removed.
 [readability/parameter_name] [5]

I agree with the style bot; better to omit this name.


More information about the webkit-reviews mailing list