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

Conversation

@0xZOne
Copy link
Member

@0xZOne 0xZOne commented Dec 4, 2021

In the FlutterActivity subclass, we can access the instance of FlutterActivityAndFragmentDelegate which is protected, as below show.

public class FlutterActivityEx extends FlutterActivity {
    private void performAttach() {
          // Attach plugins to the activity.
          // getFlutterEngine().getActivityControlSurface().attachToActivity(getActivity(), getLifecycle());
          getFlutterEngine().getActivityControlSurface().attachToActivity(delegate, getLifecycle());
    }
}

But, in FlutterFragment, the instance of FlutterActivityAndFragmentDelegate is package-level access. so we expect to modify the delegate member to protected, similar to FlutterActivity. Make sure FlutterActivity/FlutterFragment subclasses can call flutterEngine.getActivityControlSurface().attachToActivity(delegate, getLifecycle()).

Related issues: #issues/93905

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.

@0xZOne 0xZOne marked this pull request as draft December 4, 2021 11:01
@0xZOne 0xZOne marked this pull request as ready for review December 6, 2021 06:44
@0xZOne 0xZOne changed the title Make ExclusiveAppComponent accessible to FlutterActivity/FlutterFragment subclasses [android] Make ExclusiveAppComponent accessible to FlutterActivity/FlutterFragment subclasses Dec 8, 2021
// FlutterActivity and FlutterFragment. See the FlutterActivityAndFragmentDelegate
// implementation for details about why it exists.
@VisibleForTesting @Nullable /* package */ FlutterActivityAndFragmentDelegate delegate;
@VisibleForTesting @Nullable protected FlutterActivityAndFragmentDelegate delegate;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment flutter/flutter#93905 (comment)

I don't think this is the right fix

Copy link
Member Author

@0xZOne 0xZOne Dec 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated in the comments flutter/flutter#93905 (comment), there are practical demands.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead expose the method we actually need to subclasses.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we instead expose the method we actually need to subclasses.

Thank you for your reply.

In this case, I think exposing the delegate of FlutterFragment as a protected member to subclasses is the least costly solution.

@DavidMGT
Copy link

请尽快推进吧我们升级了 最新的flutter版本无法使用了

@gaoyangcr7
Copy link

关注

@0xZOne 0xZOne changed the title [android] Make ExclusiveAppComponent accessible to FlutterActivity/FlutterFragment subclasses [android] Make ExclusiveAppComponent accessible to FlutterFragment's subclasses similar to FlutterActivity's Dec 22, 2021
@Duane030414
Copy link

mark

@houziruinb87
Copy link

same issue

@chinmaygarde
Copy link
Member

It seems like @blasten and @0xZOne are in disagreement about this approach and @dnfield has a suggested workaround. Since there is not consensus among reviewers about this being the right approach, I don't think progress here is possible. Closing.

@0xZOne
Copy link
Member Author

0xZOne commented Jan 14, 2022

It seems like @blasten and @0xZOne are in disagreement about this approach and @dnfield has a suggested workaround. Since there is not consensus among reviewers about this being the right approach, I don't think progress here is possible. Closing.

I admit @dnfield's suggested workaround is more elegant. ;) I have reworked.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants