Bug 157388 - Remove unnecessary PageOverlay client function pageOverlayDestroyed
Summary: Remove unnecessary PageOverlay client function pageOverlayDestroyed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: John Wilander
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-05-05 14:23 PDT by John Wilander
Modified: 2016-05-20 14:16 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.92 KB, patch)
2016-05-05 14:34 PDT, John Wilander
no flags Details | Formatted Diff | Diff
Patch (15.33 KB, patch)
2016-05-05 17:58 PDT, John Wilander
no flags Details | Formatted Diff | Diff
Patch (16.83 KB, patch)
2016-05-06 11:07 PDT, John Wilander
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Wilander 2016-05-05 14:23:16 PDT
PageOverlay's destructor should call WebPageOverlay::pageOverlayDestroyed() and WebPageOverlay::pageOverlayDestroyed() should remove the PageOverlay from its hash map.
Comment 1 John Wilander 2016-05-05 14:24:58 PDT
rdar://problem/25471523
Comment 2 John Wilander 2016-05-05 14:34:27 PDT
Created attachment 278191 [details]
Patch
Comment 3 Tim Horton 2016-05-05 14:39:34 PDT
Comment on attachment 278191 [details]
Patch

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

> Source/WebKit2/ChangeLog:11
> +            Now revoes the given PageOverlay from its overlay HashMap.

revoes?

> Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp:-94
> -    m_client->pageOverlayDestroyed(*this);

Why did you stop calling WebPageOverlay::Client's pageOverlayDestroyed callback? Seems like you're introducing a new version of the same bug you're fixing at a different level :) (or perhaps just leaking API::PageOverlayClientImpl?)
Comment 4 John Wilander 2016-05-05 17:18:51 PDT
Changed the title to reflect what we intend to do.
Comment 5 John Wilander 2016-05-05 17:58:21 PDT
Created attachment 278212 [details]
Patch
Comment 6 Brent Fulgham 2016-05-05 21:41:33 PDT
Comment on attachment 278212 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +

You should explain what's happening:

"Remove dead PageOverlay code. All uses of the WebPageOverlay are handled through unique pointers that automatically deleted when the
WebPageOverlay object itself is deleted. Thus, there is no need to call pageOverlayDestroyed as objects are guaranteed to be completely
cleaned up."

... or something along those lines.
Comment 7 John Wilander 2016-05-06 11:07:21 PDT
Created attachment 278254 [details]
Patch
Comment 8 WebKit Commit Bot 2016-05-20 14:16:33 PDT
Comment on attachment 278254 [details]
Patch

Clearing flags on attachment: 278254

Committed r201224: <http://trac.webkit.org/changeset/201224>
Comment 9 WebKit Commit Bot 2016-05-20 14:16:37 PDT
All reviewed patches have been landed.  Closing bug.