RESOLVED FIXED 117106
[wk2] WebProcess' page-in-window count can be wrong
https://bugs.webkit.org/show_bug.cgi?id=117106
Summary [wk2] WebProcess' page-in-window count can be wrong
Tim Horton
Reported 2013-06-01 01:05:29 PDT
Interestingly: Close-tab hits both WebPage::setIsInWindow(false) and WebPage::close(). Close-window only hits WebPage::close(). I didn't add decrement-page-in-window count to WebPage::close() :( So, opening and closing a window is an easy way to make sure we never do cleanup on non-PPT. I have a patch that turns the page-in-window thing into a hashset of pageids instead of a count so we can decrement from both setIsInWindow(false) and WebPage::close.
Attachments
patch (5.82 KB, patch)
2013-06-01 01:17 PDT, Tim Horton
andersca: review+
Radar WebKit Bug Importer
Comment 1 2013-06-01 01:05:46 PDT
Tim Horton
Comment 2 2013-06-01 01:17:59 PDT
Tim Horton
Comment 3 2013-06-01 01:18:55 PDT
Comment on attachment 203483 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=203483&action=review > Source/WebKit2/ChangeLog:9 > + If we're using a single WebProcess and a window is closed, we were Maybe I should remove the bit about single WebProcess since there are cases where this can affect multiprocess too (if we're over the process cap or are using window.open or something).
Simon Fraser (smfr)
Comment 4 2013-06-01 09:51:10 PDT
Comment on attachment 203483 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=203483&action=review > Source/WebKit2/WebProcess/WebProcess.h:316 > + HashSet<uint64_t> m_inWindowPageSet; inWindowPageSet sounds like something boolean. I guess this is "set of pages in any window"?
Tim Horton
Comment 5 2013-06-02 02:22:00 PDT
(In reply to comment #4) > (From update of attachment 203483 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203483&action=review > > > Source/WebKit2/WebProcess/WebProcess.h:316 > > + HashSet<uint64_t> m_inWindowPageSet; > > inWindowPageSet sounds like something boolean. I guess this is "set of pages in any window"? It is "set of pages in this process that are parented in a window", yes.
Anders Carlsson
Comment 6 2013-06-02 17:07:30 PDT
Comment on attachment 203483 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=203483&action=review >>> Source/WebKit2/WebProcess/WebProcess.h:316 >>> + HashSet<uint64_t> m_inWindowPageSet; >> >> inWindowPageSet sounds like something boolean. I guess this is "set of pages in any window"? > > It is "set of pages in this process that are parented in a window", yes. How about m_pagesInWindows or something similar. I don’t think you need to put the type in the variable name.
Tim Horton
Comment 7 2013-06-05 12:58:03 PDT
Note You need to log in before you can comment on or make changes to this bug.