Bug 37194 – Prevent wrong use of PrintContext

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Shinichiro Hamaji]

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.

[Shinichiro Hamaji]

Created attachment 52714 [details]
Patch v1

[Shinichiro Hamaji]

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.

[Shinichiro Hamaji]

Created attachment 52716 [details]
Patch v2

[Eric Seidel (no email)]

Comment on attachment 52716 [details]
Patch v2

     if (m_isPrinting)
 43         end();
isn't needed.  the ASSERT is enough.

[Eric Seidel (no email)]

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.

[Yuzo Fujishima]

Why is m_isPrinting protected rather than private?

[Shinichiro Hamaji]

Committed r57384: <http://trac.webkit.org/changeset/57384>

[Shinichiro Hamaji]

Thanks for your suggestions! I addressed the three (2 from eric + 1 from yuzo) comments and landed the patch.