Bug 96732 – Add *explicit* keyword to constructors to WebKit2/UIProcess

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Gyuyoung Kim]

As a step of *explicit* cleanup for constructors which have only one parameter, *explicit* is added to WebKit2/UIProcess in order to avoid implicit type conversion.

[Gyuyoung Kim]

Created attachment 164058 [details]
Patch

[Gyuyoung Kim]

This patch should pass all ews except for chromium.

[Chris Dumez]

Comment on attachment 164058 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPreferences.h:83
> +    explicit WebPreferences(const WebPreferences&);

This is a copy constructor. Please don't add explicit here.

[Kentaro Hara]

Comment on attachment 164058 [details]
Patch

Marking r+ with nits:

- Please remove 'explicit' from the copy constructor that Christophe pointed out.
- Please rebase your patch.
- Please keep watching bots when landing.

[Kentaro Hara]

Comment on attachment 164058 [details]
Patch

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

> Source/WebKit2/UIProcess/WebPreferences.h:82
> +    explicit WebPreferences(const String& identifier);

Nit: 'identifier' is not needed.

[Mikhail Pozdnyakov]

Comment on attachment 164058 [details]
Patch

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

> Source/WebKit2/UIProcess/Authentication/AuthenticationDecisionListener.h:53
> +    explicit AuthenticationDecisionListener(AuthenticationChallengeProxy*);

Just wondering, does it make a lot of sense to make private constructors explicit?

[Gyuyoung Kim]

Created attachment 164257 [details]
Patch

[Gyuyoung Kim]

(In reply to comment #6)

> Just wondering, does it make a lot of sense to make private constructors explicit?

Compared to public constructor, private one seems unlikely to be used wrongly. However, in my humble opinion, to use explicit keyword is a little bit more safe than not to use it in open source case. For example, anyone can touch source code in factory functions of class inside with yoda style.

[WebKit Review Bot]

Comment on attachment 164257 [details]
Patch

Clearing flags on attachment: 164257

Committed r128683: <http://trac.webkit.org/changeset/128683>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.