WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
145030
Rename connectionDidClose and related methods to be more clear
https://bugs.webkit.org/show_bug.cgi?id=145030
Summary
Rename connectionDidClose and related methods to be more clear
Brady Eidson
Reported
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.
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
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Brady Eidson
Comment 1
2015-05-14 16:54:15 PDT
Blocking
https://bugs.webkit.org/show_bug.cgi?id=144971
Brady Eidson
Comment 2
2015-05-14 16:56:33 PDT
Created
attachment 253157
[details]
Patch v1
Brady Eidson
Comment 3
2015-05-14 17:02:48 PDT
Created
attachment 253158
[details]
Patch
Brady Eidson
Comment 4
2015-05-14 17:17:11 PDT
Created
attachment 253159
[details]
Patch v2 - Even better naming
Darin Adler
Comment 5
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.
Brady Eidson
Comment 6
2015-05-14 20:21:55 PDT
Great suggestions. Thanks for the review!
Brady Eidson
Comment 7
2015-05-14 21:41:41 PDT
http://trac.webkit.org/changeset/184370
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug