Fix comment on #5201 #5209
Merged
Fix comment on #5201 #5209
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zhongqiliang
requested review from
dahlstrom-g and
yell0wd0g
and removed request for
dahlstrom-g
kaidokert
changed the title
yell0wd0g
requested changes
Mar 27, 2025
third_party/blink/renderer/modules/cobalt/h5vcc_runtime/h_5_vcc_runtime.cc
Outdated
Show resolved
Hide resolved
zhongqiliang
force-pushed
the
warm_deep_link_fix_comment
branch
from
af4ba19 to
64257eb
Compare
zhongqiliang
force-pushed
the
warm_deep_link_fix_comment
branch
from
64257eb to
5d16dba
Compare
third_party/blink/renderer/modules/cobalt/h5vcc_runtime/h_5_vcc_runtime.h
Outdated
Show resolved
Hide resolved
|
LGTM other than my one open comment, seems to address Miguel's concerns too but I'll let him review |
|
@yell0wd0g I updated the conditions slightly in b6ad7da, ptal |
|
Tested this locally and it seems to work, though Sergei wants to do a bit more testing—will submit in a few |
andrewsavage1
approved these changes
Mar 27, 2025
yell0wd0g
approved these changes
Mar 27, 2025
third_party/blink/renderer/modules/cobalt/h5vcc_runtime/h_5_vcc_runtime.cc
Outdated
Show resolved
Hide resolved
third_party/blink/renderer/modules/cobalt/h5vcc_runtime/h_5_vcc_runtime.cc
Outdated
Show resolved
Hide resolved
third_party/blink/renderer/modules/cobalt/h5vcc_runtime/h_5_vcc_runtime.h
Outdated
Show resolved
Hide resolved
|
Tested locally after latest changes and seems to work |
cobalt-github-releaser-bot
pushed a commit
that referenced
this pull request
Mar 27, 2025
1. Corrects duplicate Mojo listener registration by removing the `RemoteAddListener()` call from `setOndeeplink()`. Registration logic is now correctly handled only within the `AddedEventListener()`, which is triggered by both property assignment and `addEventListener`. 2. Using the `DEFINE_ATTRIBUTE_EVENT_LISTENER` macro to replace `ondeeplink`/`setOndeeplink` methods. 3. Renaming `EnsureReceiverIsBound` to `EnsureRemoteIsBound`, it is for remote_h5vcc_runtime_. 4. Renaming `receiver_` to `listener_receiver_` and adding the `listener_registered_` flag for tracking AddListener 's state. https://screenshot.googleplex.com/8jeU7xBYSLq2gLN is the proof of no app crash for multiple registration of the callbacks. b/374147993 --------- Co-authored-by: Colin Liang <[email protected]> Co-authored-by: Andrew Savage <[email protected]> (cherry picked from commit f9000af)
andrewsavage1
pushed a commit
that referenced
this pull request
Mar 27, 2025
Refer to the original PR: #5209 1. Corrects duplicate Mojo listener registration by removing the `RemoteAddListener()` call from `setOndeeplink()`. Registration logic is now correctly handled only within the `AddedEventListener()`, which is triggered by both property assignment and `addEventListener`. 2. Using the `DEFINE_ATTRIBUTE_EVENT_LISTENER` macro to replace `ondeeplink`/`setOndeeplink` methods. 3. Renaming `EnsureReceiverIsBound` to `EnsureRemoteIsBound`, it is for remote_h5vcc_runtime_. 4. Renaming `receiver_` to `listener_receiver_` and adding the `listener_registered_` flag for tracking AddListener 's state. https://screenshot.googleplex.com/8jeU7xBYSLq2gLN is the proof of no app crash for multiple registration of the callbacks. b/374147993 Co-authored-by: Colin Liang <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
RemoteAddListener()call fromsetOndeeplink(). Registration logic is now correctly handled only within theAddedEventListener(), which is triggered by both property assignment andaddEventListener.DEFINE_ATTRIBUTE_EVENT_LISTENERmacro to replaceondeeplink/setOndeeplinkmethods.EnsureReceiverIsBoundtoEnsureRemoteIsBound, it is for remote_h5vcc_runtime_.receiver_tolistener_receiver_and adding thelistener_registered_flag for tracking AddListener 's state.https://screenshot.googleplex.com/8jeU7xBYSLq2gLN is the proof of no app crash for multiple registration of the callbacks.
b/374147993