Summary: | Add support for disabling/enabling termination to ChildProcess | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||
Component: | New Bugs | Assignee: | Anders Carlsson <andersca> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aroben | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Anders Carlsson
2011-04-13 14:03:57 PDT
Created attachment 89452 [details]
Patch
Comment on attachment 89452 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89452&action=review > Source/WebKit2/ChangeLog:28 > + (WebKit::PluginProcess::getSitesWithData): > + Call disableTermination()/enableTermination(). > + > + (WebKit::PluginProcess::clearSiteData): > + Ditto. Why do we need to wrap these functions in disable/enable calls? > Source/WebKit2/Shared/ChildProcess.h:39 > + void disableTermination(); > + void enableTermination(); Maybe there should be comments here instead of/in addition to the ones for m_terminationCounter. > Source/WebKit2/WebProcess/WebProcess.cpp:121 > + : ChildProcess(0.0) Can you get rid of the .0? (In reply to comment #2) > (From update of attachment 89452 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=89452&action=review > > > Source/WebKit2/ChangeLog:28 > > + (WebKit::PluginProcess::getSitesWithData): > > + Call disableTermination()/enableTermination(). > > + > > + (WebKit::PluginProcess::clearSiteData): > > + Ditto. > > Why do we need to wrap these functions in disable/enable calls? Because without a call to enableTermination, the timer will never start. > > > Source/WebKit2/Shared/ChildProcess.h:39 > > + void disableTermination(); > > + void enableTermination(); > > Maybe there should be comments here instead of/in addition to the ones for m_terminationCounter. I'll add comments. > > > Source/WebKit2/WebProcess/WebProcess.cpp:121 > > + : ChildProcess(0.0) > > Can you get rid of the .0? Yes. (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 89452 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=89452&action=review > > > > > Source/WebKit2/ChangeLog:28 > > > + (WebKit::PluginProcess::getSitesWithData): > > > + Call disableTermination()/enableTermination(). > > > + > > > + (WebKit::PluginProcess::clearSiteData): > > > + Ditto. > > > > Why do we need to wrap these functions in disable/enable calls? > > Because without a call to enableTermination, the timer will never start. So there's some case where someone will create PluginProcess that has no web process connections, will then call one of these functions, and expect the process to exit on its own? (In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 89452 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=89452&action=review > > > > > > > Source/WebKit2/ChangeLog:28 > > > > + (WebKit::PluginProcess::getSitesWithData): > > > > + Call disableTermination()/enableTermination(). > > > > + > > > > + (WebKit::PluginProcess::clearSiteData): > > > > + Ditto. > > > > > > Why do we need to wrap these functions in disable/enable calls? > > > > Because without a call to enableTermination, the timer will never start. > > So there's some case where someone will create PluginProcess that has no web process connections, will then call one of these functions, and expect the process to exit on its own? That is correct. The web process is not involved in getSitesWithData/clearSiteData. Committed r83767: <http://trac.webkit.org/changeset/83767> |