[Android] Call handleUpdateLayout even if the content didn't change

[Kerumen]

This PR fixes #11096.

I don't know enough the ReactAndroid's source code so I don't know if this is correct but I hope it is.

In a recent commit (d4b8ae7), the dispatchUpdates method now returns a boolean to dispatch or not the onLayout event. This works well but if the content is unchanged, the line nativeViewHierarchyOptimizer.handleUpdateLayout(this); is never called. I don't know if it was intended but it was this which introduces my issue. I called this again even if the content didn't change. This was the behaviour before 0.38 so I guess I didn't break anything.

Test plan (required)

I tested my pretty big app with this fix and every screen is ok.

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @astreet and @emilsjolander to be potential reviewers.

[astreet]

Why do we need to dispatch handleUpdateLayout when the layout hasn't changed? As in, what did it break for you? I have a feeling there's a better fix we can do.

[Kerumen]

I invite you to have a look at the issue I linked (#11096). I explain what worked before and broke (basically a height change that moves my button). And I created a GitHub repo with my bug.

I don't doubt there is a better fix, I'm not an expert in the React Native's java source code.

[astreet]

Thanks for the repro: I just looked through the code here and this check is just not correct. It should all be handled by hasNewLayout above. As such, the correct fix is to just combine lines 301-304 into lines 289-292 and delete this entire if-block.

[Kerumen]

Noted! I updated the PR :)

[astreet]

Awesome, thanks!

[facebook-github-bot]

@astreet has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[fred2028]

Thanks! This fixes something I was investigating

[mkonicek]

Thanks for the fix! The bug affected RN versions 0.38 and 0.39.

Your fix fb23000 will be part of the 0.40 release, and was cherry picked into 0.39, and I just now cherry-picked it into 0.38 too for people who are choosing to use 0.38 over 0.39.

[mschipperheyn]

I'm having a number of issues that seem to be related to this commit. Please reference the discussion here: #11441

[ramilushev]

I am also having an issue. See here: #11809

[astreet]

I'm having a number of issues that seem to be related to this commit. Please reference the discussion here: #11441

That problem seems to have been on iOS, this is an Android-only change.

[sahrens]

Did you experiment with calling nativeViewHierarchyOptimizer.handleUpdateLayout(this); just as often, but returning false if the layout coordinates didn't change? I think that could provide the onLayout optimization we want, but hopefully without breaking anything like the original diff did.

[sahrens]

I'm putting up a diff internally that filters out redundant onLayout calls without affecting handleUpdateLayout so hopefully that will get us some solid perf wins without breaking anything :)

[ramilushev]

@sahrens Did that internal diff make it to production?

[mkonicek]

@ramilushev Not yet but @astreet picked up the work and the diff should be in master soon.