Bug 86654 – [chromium] Clear the m_private pointer when destroying WebFilterOperations to avoid assert in WebPrivateOwnPtr

• Component: New Bugs
• Status: RESOLVED FIXED

[Dana Jansens]

[chromium] Clear the m_private pointer when destroying WebFilterOperations to avoid assert in WebPrivateOwnPtr

[Dana Jansens]

Created attachment 142298 [details]
Patch

The ::reset() method is private on other WebClasses, but I can't imagine why it would be. So I've made it private here. If this is crazy please say so!

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[Adrienne Walker]

It seems like it should be public to me.  What classes are you seeing that on?

[James Robinson]

Comment on attachment 142298 [details]
Patch

It's normally public since it makes sense for someone using a WebFilterOperations to want to get a fresh one without having to make a new object (or wrap it in a pointer or the like).

[Dana Jansens]

Created attachment 142330 [details]
Patch

K made reset() public. Dropping clear() since it's redundant then.

Adding private destroy() to clear the m_private pointer so that toFilterOperations() can remain const.

[James Robinson]

Comment on attachment 142330 [details]
Patch

reset() in WebKit API normally puts you in a null state (and there's often a bool isNull() call to check it), so it's a bit odd to have initialize() construct something non-null. Maybe leave initialize() and have ~ call reset() ?

[Dana Jansens]

I tried this at first, but the problem is toFilterOperations() is const and returns a reference, not a pointer.

So if the m_private is null, then you can't call toFilterOperations() without a segfault, because you can't change m_private.

Options:
1) Changing toFilterOperations() to non-const: breaks the WebLayer API which takes a const WebFilterOperations&.
2) Changing toFilterOperations() to return a pointer?
3) Make a static empty FilterOperations object to return when m_private is NULL?
4) Don't let m_private be NULL.

[James Robinson]

Ah, I see.  Having m_private always be non-NULL seems like the best of those options.

[James Robinson]

Comment on attachment 142330 [details]
Patch

R=me then.  Thanks for the explanation.

[Dana Jansens]

Comment on attachment 142330 [details]
Patch

Thanks!

[WebKit Review Bot]

Comment on attachment 142330 [details]
Patch

Clearing flags on attachment: 142330

Committed r117362: <http://trac.webkit.org/changeset/117362>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.