Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Re-landing Robolectric 4.8.1 #34272

Merged
merged 4 commits into from
Oct 21, 2022
Merged

Re-landing Robolectric 4.8.1 #34272

merged 4 commits into from
Oct 21, 2022

Conversation

utzcoz
Copy link
Contributor

@utzcoz utzcoz commented Jun 24, 2022 •
edited
Loading

Re-landing #33024

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@utzcoz utzcoz changed the title Fix disposeNullAndroidView with Robolectric 4.8.1 Re-landing Robolectric 4.8.1 Jun 24, 2022
@Hixie
Copy link
Contributor

Hixie commented Sep 27, 2022

@utzcoz What's the status of this PR? Were you able to determine the cause of the failures?

@utzcoz
Copy link
Contributor Author

utzcoz commented Sep 28, 2022 •
edited
Loading

@utzcoz What's the status of this PR? Were you able to determine the cause of the failures?

@Hixie Sorry, I didn't have enough time to resolve test failures when updating Robolectric and latest's Flutter Engine building on my machine. I will try it again this weekend, and I might bump Robolectric to 4.9 directly if it can be released this week.

@utzcoz
Copy link
Contributor Author

utzcoz commented Sep 30, 2022

java.lang.IllegalArgumentException: Window type mismatch. Window Context's window type is 2037, while LayoutParams' type is set to 2030. Please create another Window Context via createWindowContext(getDisplay(), 2030, null) to add window with type:2030

The presentation.show() in VirtualDisplayController throws an exception when running tests. It is the direct reason that test failed.

@utzcoz
Copy link
Contributor Author

utzcoz commented Sep 30, 2022 •
edited
Loading

Looks like https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/Presentation.java;l=228;drc=185824b521e1e883ddc947e17257e1363cb6b25f;bpv=1;bpt=1 might set different window type than SingleViewPresentation.java sets in LayoutParams in higher version. I will check whether Robolectric supports it correctly.

@utzcoz
Copy link
Contributor Author

utzcoz commented Oct 1, 2022

Looks like https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/Presentation.java;l=228;drc=185824b521e1e883ddc947e17257e1363cb6b25f;bpv=1;bpt=1 might set different window type than SingleViewPresentation.java sets in LayoutParams in higher version. I will check whether Robolectric supports it correctly.

I think that Robolectric doesn't support it correctly on higher SDK version, and I file a new issue at robolectric/robolectric#7663.

utzcoz added 4 commits October 1, 2022 12:55
@utzcoz
Bump Robolectric to 4.8.1
bbb35d7 
Bump sdk to 32.

Signed-off-by: utzcoz <[email protected]>
@utzcoz
Fix disposeNullAndroidView with Robolectric 4.8.1
24d1099 
Before Robolectric 4.8, Surface#lockHardwareCanvas will throw
exception at PlatformViewWrapper#setTexture, because Robolectric
doesn't support to shadow Surface#lockHardwareCanvas, and it
uses real Android logic with native pointer address is 0.
This failure will ensure embeddedView's parent is null, because
PlatformViewsController#createForTextureLayer will fail because
of previous mentioned error, and PlatformViewsController#createForTextureLayer
will not add embeddedView to wrapperView. So this test can pass.
From Robolectric 4.8, it supports to shadow Surface#lockHardwareCanvas
and it can pass with default true valid value, and
PlatformViewsController#createForTextureLayer will run correctly
and add embeddedView to wrapperView, and initializePlatformViewIfNeeded
will fail because embeddedView's parent is not null. So adding a new shadow
class called ShadowReleasedSurface to simulate previous
Surface#lockHardwareCanvas failure to ensure this test can work with
Robolectric 4.8 and later versions. But it is just a workaround,
the root cause is this test case depends on just-failure behavior of
Surface#lockHardwareCanvas in old Robolectric.

Signed-off-by: utzcoz <[email protected]>
@utzcoz
Add custom ShadowPresentation to avoid crash for tests
1ad4b4d 
Signed-off-by: utzcoz <[email protected]>
@utzcoz
Fix code formatting
f0bee40 
Signed-off-by: utzcoz <[email protected]>
@utzcoz utzcoz marked this pull request as ready for review October 1, 2022 05:44
@utzcoz
Copy link
Contributor Author

utzcoz commented Oct 1, 2022

Hi @Hixie , I used two custom shadows to fix test failures with Robolectric 4.8.1 to avoid changing Flutter origin logic. But I think it's better some folks can help to revisit affected tests based on proper logic/assumption of Surface. I am not familiar with Flutter internal, so I can't modify these logic quickly to meet project's requirement.

@utzcoz
Copy link
Contributor Author

utzcoz commented Oct 5, 2022

Hi @zanderso , could you help to review this PR? Thanks.

@chinmaygarde
Copy link
Member

cc @stuartmorgan for routing on #34272 (comment). Since the tests are passing, LGTM.

@stuartmorgan-g
Copy link
Contributor

I don't quite follow the explanation (perhaps because I don't have a lot of experience with Robolectric). @utzcoz Can you elaborate on what the problem in the test is exactly?

@utzcoz
Copy link
Contributor Author

utzcoz commented Oct 7, 2022

I don't quite follow the explanation (perhaps because I don't have a lot of experience with Robolectric). @utzcoz Can you elaborate on what the problem in the test is exactly?

@stuartmorgan From Robolectric 4.8.x, Robolectric implements a simple faker implmentation for Surface#lockHardwareCanvas, and it will pass without error. But looks like disposeNullAndroidView test assumes Surface#lockHardwareCanvas failed to pass test. This PR adds a new ShadowSurface to ensure Surface#lockHardwareCanvasthrows Exception as tests expected. But from 4.9, Robolectric makes Surface#lockHardwareCanvas more realistic, I think we can remove this ShadowReelaseSurface when updating Robolectric to 4.9 for Flutter/engine. cc @chinmaygarde .

@utzcoz
Copy link
Contributor Author

utzcoz commented Oct 7, 2022 •
edited
Loading

Sorry, I tried to Robolectric 4.9, looks like default ShadowSurface didn't work as expected. I will investigate whether it when I bump Robolectric to 4.9 for flutter/engine.

@utzcoz
Copy link
Contributor Author

utzcoz commented Oct 10, 2022

Hello, are there any updates for this PR? I hope it can be merged, and I can start to work for updating Robolectric to 4.9.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 21, 2022
@auto-submit auto-submit bot merged commit c7c21e5 into flutter:main Oct 21, 2022
@utzcoz utzcoz deleted the re-landing-Robolectric-4.8.1 branch October 21, 2022 17:38
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 21, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 21, 2022
@engine-flutter-autoroll
c7c21e56f Re-landing Robolectric 4.8.1 (flutter/engine#34272)
19ebaf2
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 21, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 21, 2022
@engine-flutter-autoroll
c7c21e56f Re-landing Robolectric 4.8.1 (flutter/engine#34272)
03046a3
zanderso pushed a commit to flutter/flutter that referenced this pull request Oct 21, 2022
@engine-flutter-autoroll
Roll Flutter Engine from 2b21a3125ca2 to 83092c04c105 (20 revisions) (#…
4323397 
…113847)

* e27e1964f Add touch-input-test to test_suites.yaml (flutter/engine#36900)

* b923707a3 [Impeller] remove solid stroke contents and allow strokes/vertices to use color sources (flutter/engine#36896)

* 85e4fa84d Roll Dart SDK from e06db8e1b620 to 1d418b40d8bd (1 revision) (flutter/engine#36902)

* 0b57b1cab Roll buildroot to a680bb1. (flutter/engine#36901)

* f1634aa30 Roll Fuchsia Mac SDK from IdQEnRNQNY7ZrLZ04... to jB4jUAxe89I2A-yqv... (flutter/engine#36904)

* f24ea1a04 Roll Fuchsia Linux SDK from g6-kU8so3PDiR1106... to mdl-0MUwR6uuQdKIm... (flutter/engine#36905)

* 04fa86e1b Added integration test for platform channels on windows. (flutter/engine#36853)

* 1dbf3ff76 Convert the executable directory path to UTF-8 on Windows (flutter/engine#36908)

* c3d4fc953 Roll Dart SDK from 1d418b40d8bd to f1d4c7c808bd (2 revisions) (flutter/engine#36913)

* 51b66c968 [Web] Synthesize key events for shift key on pointer events. (flutter/engine#36724)

* 584fffb67 Roll Fuchsia Mac SDK from jB4jUAxe89I2A-yqv... to fcFu9Z2KJH6oQvHnG... (flutter/engine#36919)

* 4369421b8 Roll Fuchsia Linux SDK from mdl-0MUwR6uuQdKIm... to NqPnoRHl3WYqH3SrC... (flutter/engine#36920)

* d6d38abb2 [Impeller] fix null geometry (flutter/engine#36922)

* d7f987ee3 [Impeller] Eliminate unused shader output (flutter/engine#36923)

* c255470e7 Roll Dart SDK from f1d4c7c808bd to eafe0119c9f5 (2 revisions) (flutter/engine#36925)

* 224a3def0 Restore support for building the web SDK without a prebuilt Dart SDK (flutter/engine#36926)

* c7c21e56f Re-landing Robolectric 4.8.1 (flutter/engine#34272)

* ab9802379 Roll libtess2 to 725e5e08ec8751477565f1d603fd7eb9058c277c (flutter/engine#36928)

* 95e937a44 Revert "Roll libtess2 to 725e5e08ec8751477565f1d603fd7eb9058c277c (#36928)" (flutter/engine#36932)

* 83092c04c Revert Dart SDK to 2.19.0-324.0.dev (flutter/engine#36930)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 21, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 21, 2022
@engine-flutter-autoroll
c7c21e56f Re-landing Robolectric 4.8.1 (flutter/engine#34272)
141b1f9
@zanderso zanderso mentioned this pull request Oct 25, 2022
Closed
zanderso added a commit that referenced this pull request Oct 25, 2022
@zanderso
Revert "Re-landing Robolectric 4.8.1 (#34272)"
815d276 
This reverts commit c7c21e5.
@zanderso zanderso mentioned this pull request Oct 25, 2022
Merged
zanderso added a commit that referenced this pull request Oct 26, 2022
@zanderso
Revert "Re-landing Robolectric 4.8.1 (#34272)" (#37019)
2705bcb 
This reverts commit c7c21e5.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chinmaygarde chinmaygarde chinmaygarde approved these changes

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@utzcoz @Hixie @chinmaygarde @stuartmorgan-g