Bug 156938 – Tie the DiagnosticLoggingClient's lifetime to the Page

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Chris Dumez]

Tie the DiagnosticLoggingClient's lifetime to the Page rather than to the MainFrame. The diagnostic logging client in WebKit2 requires the WebPage to be alive in order to send IPC to the UIProcess. The WebPage owns the Page and Page is not refCounted so the lifetime of the DiagnosticLoggingClient should now be tied to the one of the WebPage as well.

Previously, DiagnosticLoggingClient would stay alive as long as the MainFrame and could apparently in rare cases outlive the WebPage, thus crashing when trying to send the IPC (rdar://problem/25851499).

[Chris Dumez]

rdar://problem/25851499

[Chris Dumez]

Created attachment 277116 [details]
Patch

[Chris Dumez]

Created attachment 277123 [details]
Patch

[Chris Dumez]

Created attachment 277124 [details]
Patch

[Antti Koivisto]

Comment on attachment 277124 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277124&action=review

> Source/WebCore/page/Page.cpp:287
> +    if (m_diagnosticLoggingClient)
> +        m_diagnosticLoggingClient->pageDestroyed();

Why this instead of using std::unique_ptr for ownership?

[Antti Koivisto]

Comment on attachment 277124 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=277124&action=review

>> Source/WebCore/page/Page.cpp:287
>> +        m_diagnosticLoggingClient->pageDestroyed();
> 
> Why this instead of using std::unique_ptr for ownership?

Is there a chance that someone might access the dangling pointer during the rest of the destructor? Maybe null it?

[Chris Dumez]

Created attachment 277163 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 277163 [details]
Patch

Clearing flags on attachment: 277163

Committed r199955: <http://trac.webkit.org/changeset/199955>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.