Bug 210683 - [IPC hardening] Use MESSAGE_CHECK in WebPageProxy::loadRecentSearches() and WebPageProxy::saveRecentSearches()
Summary: [IPC hardening] Use MESSAGE_CHECK in WebPageProxy::loadRecentSearches() and W...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-17 21:02 PDT by David Kilzer (:ddkilzer)
Modified: 2020-04-19 19:18 PDT (History)
5 users (show)

See Also:


Attachments
Patch v1 (3.01 KB, patch)
2020-04-17 21:07 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2020-04-17 21:02:48 PDT
Use MESSAGE_CHECK in WebPageProxy::loadRecentSearches() and WebPageProxy::saveRecentSearches().

<rdar://problem/59240446>
Comment 1 David Kilzer (:ddkilzer) 2020-04-17 21:07:44 PDT
Created attachment 396830 [details]
Patch v1
Comment 2 David Kilzer (:ddkilzer) 2020-04-18 09:20:10 PDT
Comment on attachment 396830 [details]
Patch v1

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

> Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:73
> -    if (!name) {
> -        // FIXME: This should be a message check.
> -        return;
> -    }
> +    MESSAGE_CHECK(!name.isNull());

We could also write this like so:

    MESSAGE_CHECK(!!name);

I'm not sure if that is easier to read that !name.isNull().
Comment 3 Geoffrey Garen 2020-04-19 12:15:03 PDT
Comment on attachment 396830 [details]
Patch v1

r=me

My preference is the "!name.isNull()" style. I think it reads more clearly. Also, "!!" is kind of mind-bending if you haven't seen it before.

I believe that the "!!" style originated, especially on Windows, to work around a missing feature in C++ (a safe way to convert to boolean without converting to integer by accident). You can see vestigial remnants of the same issue in WebKit's uses of "UnspecifiedBoolType" (which is even more mind-bending). See also http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2333.html. In the modern C++ world, where operator bool can be made explicit, I believe there is no need to use "!!" anymore.
Comment 4 EWS 2020-04-19 19:18:38 PDT
Committed r260347: <https://trac.webkit.org/changeset/260347>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396830 [details].