Bug 157883

Summary: Implement operator== for WeakPtr
Product: WebKit Reporter: Rawinder Singh <rawinder.webkit>
Component: Web Template FrameworkAssignee: Rawinder Singh <rawinder.webkit>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, cdumez, cmarcelo, commit-queue, dbates, rniwa, ryanhaddad
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Archive of layout-test-results from ews103 for mac-yosemite none

Rawinder Singh
Reported 2016-05-18 21:34:55 PDT
To be able to check whether the references held by WeakPtrs are equal when using a container (e.g. WTF::HashSet), the operator== needs to be implemented.
Attachments
Patch (2.79 KB, patch)
2016-05-18 21:45 PDT, Rawinder Singh 		no flags
DetailsFormatted DiffDiff
Patch (7.39 KB, patch)
2016-05-19 21:13 PDT, Rawinder Singh 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews103 for mac-yosemite (1.07 MB, application/zip)
2016-05-19 21:58 PDT, Build Bot 		no flags
Details
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Rawinder Singh
Comment 1 2016-05-18 21:45:56 PDT
Created attachment 279346 [details] Patch
Chris Dumez
Comment 2 2016-05-18 21:55:33 PDT
Comment on attachment 279346 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=279346&action=review > Source/WTF/ChangeLog:8 > + Implement operator== for WeakPtr so that it can be used with containers (e.g. WTF::HashSet). While adding an operator==() to WeakPtr makes sense, I don't think having a HashSet<WeakPtr<X>> would be possible anyway.
Rawinder Singh
Comment 3 2016-05-18 23:56:09 PDT
(In reply to comment #2) > Comment on attachment 279346 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=279346&action=review > > > Source/WTF/ChangeLog:8 > > + Implement operator== for WeakPtr so that it can be used with containers (e.g. WTF::HashSet). > > While adding an operator==() to WeakPtr makes sense, > I don't think having a HashSet<WeakPtr<X>> would be possible anyway. Ok. Should I upload it again without the comment?
Chris Dumez
Comment 4 2016-05-19 08:58:11 PDT
(In reply to comment #3) > (In reply to comment #2) > > Comment on attachment 279346 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=279346&action=review > > > > > Source/WTF/ChangeLog:8 > > > + Implement operator== for WeakPtr so that it can be used with containers (e.g. WTF::HashSet). > > > > While adding an operator==() to WeakPtr makes sense, > > > I don't think having a HashSet<WeakPtr<X>> would be possible anyway. > > Ok. > > Should I upload it again without the comment? Yes but also, it'd be great if you used this operator in your patch as well. If you look at EventHandler.cpp, I believe you'll find places where it would be useful (e.g. m_lastScrollbarUnderMouse is weak and pointer compared in several places). Also look at EventHandler::platformCompleteWheelEvent() in EventHandlerMac.mm.
Rawinder Singh
Comment 5 2016-05-19 21:13:20 PDT
Created attachment 279465 [details] Patch
Chris Dumez
Comment 6 2016-05-19 21:51:17 PDT
Comment on attachment 279465 [details] Patch r=me
Build Bot
Comment 7 2016-05-19 21:58:46 PDT
Comment on attachment 279465 [details] Patch Attachment 279465 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1351855 Number of test failures exceeded the failure limit.
Build Bot
Comment 8 2016-05-19 21:58:50 PDT
Created attachment 279466 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Rawinder Singh
Comment 9 2016-05-20 00:04:32 PDT
(In reply to comment #6) > Comment on attachment 279465 [details] > Patch > > r=me Do you have any ideas why this patch is failing for mac?
Chris Dumez
Comment 10 2016-05-20 07:20:55 PDT
(In reply to comment #9) > (In reply to comment #6) > > Comment on attachment 279465 [details] > > Patch > > > > r=me > > Do you have any ideas why this patch is failing for mac? Mac ews has been flaky recently:/
WebKit Commit Bot
Comment 11 2016-05-20 07:41:43 PDT
Comment on attachment 279465 [details] Patch Clearing flags on attachment: 279465 Committed r201213: <http://trac.webkit.org/changeset/201213>
WebKit Commit Bot
Comment 12 2016-05-20 07:41:48 PDT
All reviewed patches have been landed. Closing bug.
Ryan Haddad
Comment 13 2016-05-20 09:35:29 PDT
This change appears to have cause an API test failure on Mac and iOS" FAIL WTF_WeakPtr.MultipleFactories /Volumes/Data/slave/elcapitan-release/build/Tools/TestWebKitAPI/Tests/WTF/WeakPtr.cpp:86 Value of: weakPtr1 != weakPtr2 Actual: false Expected: true <https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/6206/steps/run-api-tests/logs/stdio>
Chris Dumez
Comment 14 2016-05-20 10:01:52 PDT
(In reply to comment #13) > This change appears to have cause an API test failure on Mac and iOS" > > FAIL WTF_WeakPtr.MultipleFactories > > /Volumes/Data/slave/elcapitan-release/build/Tools/TestWebKitAPI/Tests/WTF/ > WeakPtr.cpp:86 > Value of: weakPtr1 != weakPtr2 > Actual: false > Expected: true > > <https://build.webkit.org/builders/ > Apple%20El%20Capitan%20Release%20WK2%20%28Tests%29/builds/6206/steps/run-api- > tests/logs/stdio> Fixed in <http://trac.webkit.org/changeset/201214>.
Note You need to log in before you can comment on or make changes to this bug.