Bug 117106

Summary: [wk2] WebProcess' page-in-window count can be wrong
Product: WebKit Reporter: Tim Horton <thorton>
Component: WebKit2Assignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch andersca: review+

Description Tim Horton 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.
Comment 1 Radar WebKit Bug Importer 2013-06-01 01:05:46 PDT
<rdar://problem/14040991>
Comment 2 Tim Horton 2013-06-01 01:17:59 PDT
Created attachment 203483 [details]
patch
Comment 3 Tim Horton 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).
Comment 4 Simon Fraser (smfr) 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"?
Comment 5 Tim Horton 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.
Comment 6 Anders Carlsson 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.
Comment 7 Tim Horton 2013-06-05 12:58:03 PDT
http://trac.webkit.org/changeset/151235