Summary: | Convert WebProcess over to using disableTermination/enableTermination | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Anders Carlsson <andersca> | ||||
Component: | New Bugs | Assignee: | Anders Carlsson <andersca> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Other | ||||||
OS: | OS X 10.5 | ||||||
Attachments: |
|
Description
Anders Carlsson
2011-04-13 14:58:32 PDT
Created attachment 89470 [details]
Patch
Comment on attachment 89470 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89470&action=review It's a little confusing that all the terminateIfPossible calls have turned into a temporary disabling of termination. > Source/WebKit2/WebProcess/WebProcess.cpp:505 > void WebProcess::removeWebPage(uint64_t pageID) > { > m_pageMap.remove(pageID); > - terminateIfPossible(); > + > + enableTermination(); Do we need to verify that pageID was in m_pageMap? > Source/WebKit2/WebProcess/WebProcess.cpp:733 > + WebProcess::LocalTerminationDisabler terminationDisabler(WebProcess::shared()); No need for either of the "WebProcess::"s. > Source/WebKit2/WebProcess/WebProcess.cpp:755 > + WebProcess::LocalTerminationDisabler terminationDisabler(WebProcess::shared()); Or here. > Source/WebKit2/WebProcess/WebCoreSupport/WebDatabaseManager.cpp:158 > + ChildProcess::LocalTerminationDisabler terminationDisabler(WebProcess::shared()); Why ChildProcess and not WebProcess? (In reply to comment #2) > (From update of attachment 89470 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=89470&action=review > > It's a little confusing that all the terminateIfPossible calls have turned into a temporary disabling of termination. > > > Source/WebKit2/WebProcess/WebProcess.cpp:505 > > void WebProcess::removeWebPage(uint64_t pageID) > > { > > m_pageMap.remove(pageID); > > - terminateIfPossible(); > > + > > + enableTermination(); > > Do we need to verify that pageID was in m_pageMap? > Not really, but I added an assertion. > > Source/WebKit2/WebProcess/WebProcess.cpp:733 > > + WebProcess::LocalTerminationDisabler terminationDisabler(WebProcess::shared()); > > No need for either of the "WebProcess::"s. changed WebProcess::shared() to *this. > > > Source/WebKit2/WebProcess/WebProcess.cpp:755 > > + WebProcess::LocalTerminationDisabler terminationDisabler(WebProcess::shared()); > > Or here. > > > Source/WebKit2/WebProcess/WebCoreSupport/WebDatabaseManager.cpp:158 > > + ChildProcess::LocalTerminationDisabler terminationDisabler(WebProcess::shared()); > > Why ChildProcess and not WebProcess? Changed to WebProcess. Committed r83774: <http://trac.webkit.org/changeset/83774> |