Bug 156080 - Page overlay tests sometimes crash under MockPageOverlayClient::uninstallAllOverlays()
Summary: Page overlay tests sometimes crash under MockPageOverlayClient::uninstallAllO...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-03-31 13:08 PDT by Alexey Proskuryakov
Modified: 2016-04-01 09:38 PDT (History)
4 users (show)

See Also:

Attachments
proposed fix (2.87 KB, patch)
2016-03-31 13:11 PDT, Alexey Proskuryakov 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2016-03-31 13:08:27 PDT 
rdar://problem/24922183
Comment 1 Alexey Proskuryakov 2016-03-31 13:11:46 PDT 
Created attachment 275314 [details]
proposed fix
Comment 2 Alex Christensen 2016-03-31 19:39:09 PDT 
Comment on attachment 275314 [details]
proposed fix

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

r=me
I would be ok with removing the dead code in this patch.

> Source/WebCore/testing/MockPageOverlayClient.cpp:78
> +    // FIXME: This is dead code, nothing ever calls this function. It's not clear to me what the intention was,

Let's remove this, then!
Comment 3 Alexey Proskuryakov 2016-03-31 21:02:14 PDT 
Comment on attachment 275314 [details]
proposed fix

There is a lot more code to review, or possibly to fix. Out of scope for this patch.
Comment 4 WebKit Commit Bot 2016-03-31 21:53:59 PDT 
Comment on attachment 275314 [details]
proposed fix

Clearing flags on attachment: 275314

Committed r198940: <http://trac.webkit.org/changeset/198940>
Comment 5 WebKit Commit Bot 2016-03-31 21:54:02 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 6 Darin Adler 2016-04-01 09:36:06 PDT 
To me, removing an unused function seems even better than adding a comment to it.
Comment 7 Alexey Proskuryakov 2016-04-01 09:38:26 PDT 
Normally, I would agree, but not in this case, where it's more likely correct to make the function used. rdar://problem/25471523 tracks that.