We can misuse PrintContext in several ways. We can call PrintContext::begin() twice, we can call PrintContext::end() without PrintContext::begin(), and we can forget PrintContext::end() after PrintContext::begin(). Bug 36049 is an example of such mistakes. Let's add a boolean flag to prevent such misuses.
Created attachment 52714 [details] Patch v1
Comment on attachment 52714 [details] Patch v1 > PrintContext::~PrintContext() > { > + if (m_isPrinting) > + end(); > m_pageRects.clear(); > } Or, we may want to add ASSERT(!m_isPrinting) here. I personally like this kind of automatic release, but I'm not sure if this is suitable for this case. If a user of PrintContext relies on this, her/his code may have begin() without end(), which might look weird a bit. Hmm... I changed my mind while I'm writing this comment. I'll revise the patch.
Created attachment 52716 [details] Patch v2
Comment on attachment 52716 [details] Patch v2 if (m_isPrinting) 43 end(); isn't needed. the ASSERT is enough.
Comment on attachment 52716 [details] Patch v2 You should consider documenting what m_isPrinting is for in teh header, otherwise this looks OK (- the release-only auto-end stuff). r=me with the two above comments addressed.
Why is m_isPrinting protected rather than private?
Committed r57384: <http://trac.webkit.org/changeset/57384>
Thanks for your suggestions! I addressed the three (2 from eric + 1 from yuzo) comments and landed the patch.