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

Android: Made sure we get the initial route from the intent. #40148

Merged
merged 1 commit into from
Mar 9, 2023

Conversation

gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Mar 8, 2023

fixes b/271100292

Edit: Fixes flutter/flutter#121244.

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.

@gaaclarke
Copy link
Member Author

attn @math1man

@camsim99
Copy link
Contributor

camsim99 commented Mar 8, 2023

Oh I fixed this issue here but I did something different I think because I thought the cause was different: #40119. Does this fix it? There's a repro here flutter/flutter#121244

@math1man
Copy link
Contributor

math1man commented Mar 8, 2023

This LGTM. I think this is a more consistent and efficient fix than #40119

@gaaclarke
Copy link
Member Author

This fix was based off of the debugging that @math1man did. His analysis makes sense to me although @camsim99 will probably have a better sense than me about the different flows through this code.

Assuming that this works, this fix seems preferable. @math1man made a good point that there are some temporal dependencies about when you an access the activity so making sure our scenario apps pass on ci will be important. It would be nice if he could test the fix in his app just to make sure.

@math1man
Copy link
Contributor

math1man commented Mar 8, 2023

I've verified this fix locally in my app and confirmed it resolves the issue.

I would say that my biggest reason to prefer this over #40119 is that it is closer to what the behavior was before #38367 introduced this regression.

@camsim99
Copy link
Contributor

camsim99 commented Mar 8, 2023

cc @reidbaker

Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

Lgtm!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android] Flutter does not get the correct initial route when launch by a deep link
3 participants