Remove stale Background Fetch cancellation TODOs
YelihiChromium Background Fetch에 남아 있던 불필요한 TODO를 제거한 기여
문제 설명
Background Fetch에는 다운로드 취소 이후 이미 예약된 callback이 전달될 수 있는 상황과 관련된 TODO가 남아 있었습니다.
이 문제를 해결하기 위해 이전 CL에서는 ControllerImpl에서 pending callback과 취소 상태를 별도로 관리하는 방식을 구현했습니다.
하지만 코드 리뷰 과정에서 다음과 같은 의견을 받았습니다.
(관련 포스팅 : Don't ignore CancelDownload() while a completion callback is pending)[https://ossca-chromium.github.io/contributions/patches/8277215]
"Can the client simply filter out or ignore late callbacks on their end after cancelling, rather than having ControllerImpl intercept and rewrite in-flight tasks? Background Fetch already does this."
리뷰어의 의견을 바탕으로 다시 코드를 확인한 결과, Background Fetch Client에서는 이미 자신의 download GUID를 관리하고 있었고 취소된 다운로드의 late callback을 lookup 과정에서 무시할 수 있었습니다.
해결 내용
기존 Client의 동작만으로 취소 이후 전달되는 callback을 처리할 수 있기 때문에 ControllerImpl에 별도의 상태를 추가할 필요가 없음을 확인했습니다.
따라서 새로운 로직을 추가하는 대신, 더 이상 유효하지 않은 Background Fetch의 TODO를 제거했습니다.
테스트 방법
실제 동작을 변경하는 것이 아니라 기존 Client에서 이미 처리하고 있는 동작을 확인한 뒤 TODO를 제거하는 변경이므로 별도의 테스트 코드는 추가하지 않았습니다.
기존 Background Fetch Client의 GUID lookup 로직을 확인하여 취소된 다운로드의 late callback이 무시되는 것을 확인했습니다.
배운 점
처음에는 문제가 발생하는 ControllerImpl에서 새로운 상태를 추가하는 방향으로 접근했지만, 리뷰를 통해 문제를 해결하는 것뿐만 아니라 어느 계층이 해당 책임을 가져야 하는지 확인하는 것이 중요하다는 점을 배웠습니다.
또한 새로운 로직을 추가하기 전에 기존 코드베이스에 동일한 문제를 이미 처리하고 있는 로직이 있는지 충분히 확인해야 한다는 점을 경험했습니다.