RecyclerView invokes adapter onBind while target view to be bound is temporarily detached [265347515]

Fixed

Bug

Status Update

No update yet.

Description

ry...@google.com

created issue #1

Jan 12, 2023 11:02PM

Quoting Adam Powell on an internal issue:

When we investigated [a crash with Compose in a Google app] it looks like something between RecyclerView/LinearLayoutManager is leaving the view in a temporarily detached state via ViewGroup.detachViewFromParent during the bind operation, which will disrupt the functioning of many Jetpack libraries that use ViewTreeFoo service locators to discover dependencies, Compose included. We should definitely work to fix this so that we only invoke binding operations from adapters while the view is either fully attached or fully detached.

Comments

ry...@google.com <ry...@google.com> #2Jan 12, 2023 11:03PM

since these are in public API (:/) we need to do this in 1.2

ap...@google.com <ap...@google.com> #3Mar 2, 2023 03:35AM

Marked as fixed.

since it is already marked as deprecated, we can probably do it by now.

hu...@google.com <hu...@google.com> #4Mar 6, 2023 08:06PM

Opening diff shortly

hu...@google.com <hu...@google.com> #5Mar 6, 2023 08:08PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit d576cbdc911cba16638a44fd8223391a90a07ef7
Author: Mike Nakhimovich <digitalbuddha@users.noreply.github.com>
Date: Tue Aug 11 09:30:34 2020

[GH] Hide deprecated internal API.

## Proposed Changes

* `RoomDatabase.java` has protected `mCallbacks` field which is leaking in the API docs, we should @Hide it.

## Testing

Test: Ran unit tests locally

## Issues Fixed

Fixes: 76109329

This is an imported pull request from

https://github.com/androidx/androidx/pull/61.

Resolves #61
Github-Pr-Head-Sha: 6440daa3a63752c7f9d5ba2a390248cd85bc634f
GitOrigin-RevId: fe92d8466a59b44b218b6ca3cbd57dcda17992f7

Change-Id: Id599cdf5b02b32bdae0166266fb7da967598fe92

A room/runtime/api/current.ignore
M room/runtime/api/current.txt
M room/runtime/api/public_plus_experimental_current.txt
M room/runtime/api/restricted_current.txt
M room/runtime/src/main/java/androidx/room/RoomDatabase.java

https://android-review.googlesource.com/1396827

ry...@google.com <ry...@google.com> #6Mar 7, 2023 01:09AM

It looks like this code in ViewPager2 is a workaround for the bug that's fixed in this change.

Specifically, it looks to be working around the fact that onBindViewHolder was previously called with temporarily detached views, and using this listener to wait for it to be properly attached so that it can perform operations that rely on it being properly attached.

The previous state was bad precisely because it forced hacky workarounds like this due to the lack of a proper way to listen for changes in temp-detached state (there are listeners, but they don't work, and would likely require changes in both RecyclerView and framework to make them work...it's a very flawed design). Until seeing this workaround, I wasn't aware it was even possible at all to figure it out (and even this has some limitations that probably aren't really relevant to VP2's use-case).

I think the correct solution here is to remove this workaround in ViewPager2 and release new RecyclerView and ViewPager2 versions in parallel to address this, with release notes advising users to upgrade. Jakub, what do you think?

(also, I think I vaguely recall hearing that it's possible to do some sort of Gradle module metadata thing where we can also add the information that VP2 should be at least a certain version if being used with a given RV version? I'm not sure if/how we could do that, though)

Message last modified on Mar 7, 2023 01:14AM

hu...@google.com <hu...@google.com> #7Mar 7, 2023 03:36PM

Thanks for the explanation!

~~I'm wondering if there will be versioning issues re. Android platform support vs. AndroidX?~~ (NVM, this is all AndroidX).

Message last modified on Mar 7, 2023 03:47PM

ap...@google.com <ap...@google.com> #8Apr 27, 2023 06:58PM

Project: platform/frameworks/support
Branch: androidx-main

commit e8d8836df99cf5e0d1d4bc318947d89d4103c569
Author: Ryan Mentley <ryanmentley@google.com>
Date: Wed Mar 08 18:26:56 2023

Ensure that ViewPager2 is on the latest version when used with an updated recyclerview

As discussed at

https://issuetracker.google.com/issues/265347515#comment4,
ViewPager2 had a workaround for that bug that now crashes when the bug is
not present. To fix this, we need to ensure that ViewPager2 is on a
version containing the corresponding fix in ViewPager2.

Bug: 265347515
Bug: 271618925
Test: None (TBD?)
Change-Id: I18a5140755c15a0c93e2647207152b679e463233

M recyclerview/recyclerview/build.gradle

https://android-review.googlesource.com/2478185

ju...@google.com <ju...@google.com> #9May 24, 2023 07:37PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.recyclerview:recyclerview:1.3.1-rc01

pr...@google.com <pr...@google.com> #10Oct 18, 2023 08:42PM