Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Explicit ImageList ownership management" (servicing) #4314

Merged
merged 1 commit into from
Dec 9, 2020

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Dec 2, 2020

This reverts commit 03db3fb.

Resolves #4169
Resolves #4275

Proposed changes

Revert "Explicit ImageList ownership management. (#3601)"
This reverts commit 03db3fb.

Revert "Fix ListView no longer displays images (#4184)"
This reverts commit d0608e7.

We have observed an instability of tests under stress (and reasonably high degree of concurrency) presumably caused by ImageList lifetime handling (notably #3358).
The changes introduced in #3601 have helped with tests stability, however resulted in a number of regressions, such as #4169 and #4275.

Restore the original implementation given it worked reasonably well for the past so many years.

Customer Impact

  • The ImageList behaviours should be the same as in .NET Framework and .NET Core.

Regression?

  • Yes

Risk

  • Minimal
Microsoft Reviewers: Open in CodeFlow

Revert "Explicit ImageList ownership management. (dotnet#3601)"
This reverts commit 03db3fb.

Revert "Fix `ListView` no longer displays images (dotnet#4184)"
This reverts commit d0608e7.

We have observed an instability of tests under stress (and reasonably
high degree of concurrency) presumably caused by ImageList lifetime
handling (notably dotnet#3358).
The changes introduced in dotnet#3601 have helped with tests stability,
however resulted in a number of regressions, such as dotnet#4169 and dotnet#4275.

Restore the original implementation given it worked reasonably well for
the past so many years.

(cherry picked from commit b0ee5da)
@RussKie RussKie added the Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria label Dec 2, 2020
@RussKie RussKie added this to the 5.0.2 milestone Dec 2, 2020
@RussKie RussKie requested a review from a team as a code owner December 2, 2020 23:21
@ghost ghost assigned RussKie Dec 2, 2020
@codecov
Copy link

codecov bot commented Dec 2, 2020

Codecov Report

Merging #4314 (005055d) into release/5.0 (200d740) will decrease coverage by 0.00539%.
The diff coverage is 57.57576%.

@@                  Coverage Diff                  @@
##           release/5.0       #4314         +/-   ##
=====================================================
- Coverage     68.10735%   68.10196%   -0.00539%     
=====================================================
  Files             1419        1419                 
  Lines           510152      510022        -130     
  Branches         41553       41531         -22     
=====================================================
- Hits            347451      347335        -116     
+ Misses          156406      156396         -10     
+ Partials          6295        6291          -4     
Flag Coverage Δ
Debug 68.10196% <57.57576%> (-0.00539%) ⬇️
production 37.30519% <56.25000%> (-0.02690%) ⬇️
test 97.91921% <100.00000%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@leecow leecow added Servicing-approved .NET Shiproom approved the PR for merge and removed Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Dec 3, 2020
@RussKie RussKie merged commit cff5155 into dotnet:release/5.0 Dec 9, 2020
@RussKie RussKie deleted the fix_4169_revert5 branch December 9, 2020 05:20
@ghost ghost locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Servicing-approved .NET Shiproom approved the PR for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants