Bug 145030 - Rename connectionDidClose and related methods to be more clear
Summary: Rename connectionDidClose and related methods to be more clear
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 144971
  Show dependency treegraph
 
Reported: 2015-05-14 16:53 PDT by Brady Eidson
Modified: 2015-05-14 21:41 PDT (History)
2 users (show)

See Also:


Attachments
Patch v1 (16.03 KB, patch)
2015-05-14 16:56 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (15.99 KB, patch)
2015-05-14 17:02 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch v2 - Even better naming (15.94 KB, patch)
2015-05-14 17:17 PDT, Brady Eidson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-05-14 16:53:57 PDT
Rename connectionDidClose and related methods to be more clear

When reading code in the UIProcess, it is easy to confuse "connectionDidClose(IPC::Connection)" and "didClose(IPC::Connection)"

The former means "the UI Process is purposefully shutting down the connection to a child process.

The later means "the remote end point of this connection has gone away"

We should leave the later, but change the former to be much more explicit.
Comment 1 Brady Eidson 2015-05-14 16:54:15 PDT
Blocking https://bugs.webkit.org/show_bug.cgi?id=144971
Comment 2 Brady Eidson 2015-05-14 16:56:33 PDT
Created attachment 253157 [details]
Patch v1
Comment 3 Brady Eidson 2015-05-14 17:02:48 PDT
Created attachment 253158 [details]
Patch
Comment 4 Brady Eidson 2015-05-14 17:17:11 PDT
Created attachment 253159 [details]
Patch v2 - Even better naming
Comment 5 Darin Adler 2015-05-14 19:45:53 PDT
Comment on attachment 253159 [details]
Patch v2 - Even better naming

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

The phrase “web process shutting down’ seems to be missing a verb; the tense is confusing, like intentional strangeness of the movie title Rachel Getting Married.

How about “web process will shut down” or “web process is shutting down” or “web process did shut down”? Or whatever the actual situation is.

> Source/WebKit2/Shared/ChildProcessProxy.h:76
> +    void shutdownProcess();

The verb is two words “shut down”, so this should be shutDownProcess.

> Source/WebKit2/UIProcess/Databases/DatabaseProcessProxy.cpp:77
> +    ASSERT(this->connection() == &connection);

This should use ASSERT_UNUSED.

> Source/WebKit2/UIProcess/Network/NetworkProcessProxy.cpp:100
> +    ASSERT(this->connection() == &connection);

This should use ASSERT_UNUSED.

> Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp:87
> +    ASSERT(this->connection() == &connection);

This should use ASSERT_UNUSED.

> Source/WebKit2/UIProcess/WebProcessProxy.h:164
> +    void shutdown();

Again, the verb “shut down” is two words and this should be shutDown.
Comment 6 Brady Eidson 2015-05-14 20:21:55 PDT
Great suggestions. Thanks for the review!
Comment 7 Brady Eidson 2015-05-14 21:41:41 PDT
http://trac.webkit.org/changeset/184370