Make ObserverList non reentrant by default. |
|
Issue descriptionI'll run through bots and update the bug with the result.
,
Feb 21 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f1f6a67fe0951409e904022e4895a5458fa60b2 commit 6f1f6a67fe0951409e904022e4895a5458fa60b2 Author: Kinuko Yasuda <kinuko@chromium.org> Date: Wed Feb 21 05:48:04 2018 Revert "Add allow_reentrancy parameter to ObserverList" This reverts commit 7a30eec9fe13e9f5c91b2373fe96871c1e16ad3c. Reason for revert: makes compile on WinMSVC64 fail. FAILED: obj/base/base_unittests/observer_list_unittest.obj ninja -t msvc -e environment.x64 -- C:\b\c\goma_client/gomacc.exe <snip> /c ../../base/observer_list_unittest.cc /Foobj/base/base_unittests/observer_list_unittest.obj /Fd"obj/base/base_unittests_cc.pdb" c:\b\c\b\win\src\base\observer_list_unittest.cc(1269) : error C2220: warning treated as error - no 'object' file generated c:\b\c\b\win\src\base\observer_list_unittest.cc(1269) : warning C4702: unreachable code Examples: https://ci.chromium.org/buildbot/chromium.win/WinMSVC64%20%28dbg%29/3127 https://ci.chromium.org/buildbot/chromium.win/WinMSVC64%20%28dbg%29/3115 The fix should be probably a one-liner while it wasn't too obvious, so let me revert... sorry for that. Original change's description: > Add allow_reentrancy parameter to ObserverList > > Certain client code wants to make sure that no loop is > performed while looping to avoid unexpected behaivor. > Use this new parameter to check this condition in debug build & unit tests. > > Introduced ReentrantObserverList (which is currently same as defualt) > to replace the ones that indeed have to be reentrant in a separate CL. > > BUG=812109 > TEST=Covered by unittests > > Change-Id: Idcf2ed59215ad9d3d8da278d15eddfcd2dedffa9 > Reviewed-on: https://chromium-review.googlesource.com/915441 > Commit-Queue: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Daniel Cheng <dcheng@chromium.org> > Reviewed-by: Wez <wez@chromium.org> > Cr-Commit-Position: refs/heads/master@{#537949} TBR=dcheng@chromium.org,oshima@chromium.org,wez@chromium.org Change-Id: I4a98de69772474f8438087a93a310fa68a88525f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 812109 Reviewed-on: https://chromium-review.googlesource.com/928061 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#538028} [modify] https://crrev.com/6f1f6a67fe0951409e904022e4895a5458fa60b2/base/observer_list.h [modify] https://crrev.com/6f1f6a67fe0951409e904022e4895a5458fa60b2/base/observer_list_unittest.cc
,
Feb 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/538b1db48679ff36ff68c3db02c9c22648fe82be commit 538b1db48679ff36ff68c3db02c9c22648fe82be Author: Mitsuru Oshima <oshima@chromium.org> Date: Wed Feb 28 04:05:23 2018 [reland] Add allow_reentrancy parameter to ObserverList Certain client code wants to make sure that no loop is performed while looping to avoid unexpected behaivor. Use this new parameter to check this condition in debug build & unit tests. Introduced ReentrantObserverList (which is currently same as defualt) to replace the ones that indeed have to be reentrant in a separate CL. BUG=812109 TEST=Covered by unittests Reviewed-on: https://chromium-review.googlesource.com/915441 Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Wez <wez@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#537949} Change-Id: Id4e3d04802f247491b39585219482648733b2563 Reviewed-on: https://chromium-review.googlesource.com/939530 Cr-Commit-Position: refs/heads/master@{#539698} [modify] https://crrev.com/538b1db48679ff36ff68c3db02c9c22648fe82be/base/observer_list.h [modify] https://crrev.com/538b1db48679ff36ff68c3db02c9c22648fe82be/base/observer_list_unittest.cc
,
Mar 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/338eaef9c75857f87e37bfea36b87d7bbc64c49a commit 338eaef9c75857f87e37bfea36b87d7bbc64c49a Author: Mitsuru Oshima <oshima@chromium.org> Date: Thu Mar 29 18:52:26 2018 Use ReentrantObserverList for aura::Window::observers_ BUG=812109 TEST=aura_unittests pass with "allow_reentrancy=false", which will be default in the future. Change-Id: Ic1ab64cbf4021f4495933bb4803130c1715b203f Reviewed-on: https://chromium-review.googlesource.com/984831 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#546901} [modify] https://crrev.com/338eaef9c75857f87e37bfea36b87d7bbc64c49a/ui/aura/window.h [modify] https://crrev.com/338eaef9c75857f87e37bfea36b87d7bbc64c49a/ui/aura/window_port.cc [modify] https://crrev.com/338eaef9c75857f87e37bfea36b87d7bbc64c49a/ui/aura/window_port.h
,
Mar 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2ed30ffa70000045d888892851a3e7d69b181a7b commit 2ed30ffa70000045d888892851a3e7d69b181a7b Author: Findit <findit-for-me@appspot.gserviceaccount.com> Date: Thu Mar 29 20:02:11 2018 Revert "Use ReentrantObserverList for aura::Window::observers_" This reverts commit 338eaef9c75857f87e37bfea36b87d7bbc64c49a. Reason for revert: Findit (https://goo.gl/kROfz5) identified CL at revision 546901 as the culprit for failures in the build cycles as shown on: https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzMzOGVhZWY5Yzc1ODU3Zjg3ZTM3YmZlYTM2Yjg3ZDdiYmM2NGM0OWEM Sample Failed Build: https://ci.chromium.org/buildbot/chromium/Win/65521 Sample Failed Step: compile Original change's description: > Use ReentrantObserverList for aura::Window::observers_ > > BUG=812109 > TEST=aura_unittests pass with "allow_reentrancy=false", which will > be default in the future. > > Change-Id: Ic1ab64cbf4021f4495933bb4803130c1715b203f > Reviewed-on: https://chromium-review.googlesource.com/984831 > Reviewed-by: Scott Violet <sky@chromium.org> > Commit-Queue: Mitsuru Oshima <oshima@chromium.org> > Cr-Commit-Position: refs/heads/master@{#546901} No-Presubmit: true No-Tree-Checks: true No-Try: true BUG=812109 Change-Id: I42ac5f5dc01e24f392984c3f77a0a3f4aae636e3 Reviewed-on: https://chromium-review.googlesource.com/986892 Cr-Commit-Position: refs/heads/master@{#546920} [modify] https://crrev.com/2ed30ffa70000045d888892851a3e7d69b181a7b/ui/aura/window.h [modify] https://crrev.com/2ed30ffa70000045d888892851a3e7d69b181a7b/ui/aura/window_port.cc [modify] https://crrev.com/2ed30ffa70000045d888892851a3e7d69b181a7b/ui/aura/window_port.h
,
Dec 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01e8130c0824918abd6fe95af68d6a5100535b07 commit 01e8130c0824918abd6fe95af68d6a5100535b07 Author: Mitsuru Oshima <oshima@chromium.org> Date: Mon Dec 10 19:30:47 2018 Use ReentrantObserverList for aura::Window::observers_ BUG=812109 TEST=aura_unittests pass with "allow_reentrancy=false", which will be default in the future. Change-Id: I22cfc645bb15bfb7e9d6dc08c372124b216738a4 Reviewed-on: https://chromium-review.googlesource.com/c/1171372 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#615202} [modify] https://crrev.com/01e8130c0824918abd6fe95af68d6a5100535b07/ui/aura/window.h [modify] https://crrev.com/01e8130c0824918abd6fe95af68d6a5100535b07/ui/aura/window_port.cc [modify] https://crrev.com/01e8130c0824918abd6fe95af68d6a5100535b07/ui/aura/window_port.h |
|
►
Sign in to add a comment |
|
Comment 1 by bugdroid1@chromium.org
, Feb 21 2018