Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add DeepLinkListener. #5004

Merged
merged 8 commits into from
Mar 6, 2025
Merged

Add DeepLinkListener. #5004

merged 8 commits into from
Mar 6, 2025

Conversation

zhongqiliang
Copy link
Contributor

@zhongqiliang zhongqiliang commented Mar 3, 2025 •
edited
Loading

  1. Introduce DeepLinkListener for bidirectional Mojo communication.
  2. Add listeners created by H5vccRuntimeImpl to DeepLinkManager::listeners_.
  3. Integrate the implementation with starboard_bridge.cc to handle warm-start deep links.

b/374147993

@zhongqiliang zhongqiliang changed the title Add onDeepLink EventHandler to h5vcc_runtime IDL Add DeepLinkListener. Mar 4, 2025
@zhongqiliang zhongqiliang force-pushed the add_deeplink_listener branch from 3c52190 to 13a4a62 Compare March 5, 2025 19:35
@zhongqiliang zhongqiliang marked this pull request as ready for review March 5, 2025 21:36
@zhongqiliang zhongqiliang requested a review from a team as a code owner March 5, 2025 21:36
@zhongqiliang zhongqiliang requested review from briantting, yell0wd0g, kaidokert, hlwarriner and andrewsavage1 and removed request for briantting March 5, 2025 21:36
yell0wd0g
yell0wd0g requested changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@yell0wd0g yell0wd0g left a comment

Choose a reason for hiding this comment

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

Looking good, I have some questions on when the listener gets notified.

@zhongqiliang zhongqiliang requested a review from yell0wd0g March 6, 2025 19:32
Colin Liang added 2 commits March 6, 2025 13:23
yell0wd0g
yell0wd0g approved these changes Mar 6, 2025
View reviewed changes
Copy link
Contributor

@yell0wd0g yell0wd0g left a comment

Choose a reason for hiding this comment

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

LGTM, considering that the WPTs are not working yet.

@zhongqiliang zhongqiliang enabled auto-merge March 6, 2025 22:07
@zhongqiliang zhongqiliang disabled auto-merge March 6, 2025 22:07
@zhongqiliang zhongqiliang enabled auto-merge (squash) March 6, 2025 22:07
@zhongqiliang zhongqiliang merged commit 47f6f54 into youtube:main Mar 6, 2025
137 checks passed
@zhongqiliang zhongqiliang deleted the add_deeplink_listener branch March 6, 2025 22:16
johnxwork added a commit that referenced this pull request Mar 7, 2025
@johnxwork
Revert "Add DeepLinkListener. (#5004)"
529eb36 
This reverts commit 47f6f54.
@johnxwork johnxwork mentioned this pull request Mar 7, 2025
Merged
johnxwork added a commit that referenced this pull request Mar 8, 2025
@johnxwork
Revert "Add DeepLinkListener." (#5035)
a3ab0e9 
Reverts #5004

b/401383858
@yell0wd0g yell0wd0g mentioned this pull request Mar 26, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yell0wd0g yell0wd0g yell0wd0g approved these changes

@kaidokert kaidokert Awaiting requested review from kaidokert

@hlwarriner hlwarriner Awaiting requested review from hlwarriner

@andrewsavage1 andrewsavage1 Awaiting requested review from andrewsavage1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@zhongqiliang @yell0wd0g