This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Fix handler unregistration in C++ channels #16794
Merged
Conversation
This file contains hidden or 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
MethodChannel and BasicMessageChannel in the C++ wrapper didn't have the expected semantics that passing a null handler would remove any existing handler. This was inconsistent with other platforms and with the lower-level object APIs in this wrapper, and made unregistration in cases where that's desirable more difficult due to needing to keep other object references. Adds tests for this functionality, and some backfill of missing tests for basic handler behavior. See flutter/flutter#51207
franciscojma86
approved these changes
Feb 25, 2020
|
|
||
| namespace { | ||
|
|
||
| class TestBinaryMessenger : public BinaryMessenger { |
Contributor
There was a problem hiding this comment.
Would TestBinaryMessenger be better named as MockBinaryMessenger, and use the Test prefix for the subjects being tests?
Contributor
Author
There was a problem hiding this comment.
Since it's manual, Fake might be better than Mock?
I'm going to land as-is for now, since at least the registrar unit test already uses this name, and we can revisit naming for everything at once later.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Feb 26, 2020
dnfield
pushed a commit
to flutter/flutter
that referenced
this pull request
Feb 26, 2020
* 05dd023 Roll src/third_party/dart 73f6d15665a3..7aa824076c34 (11 commits) (flutter/engine#16780) * 9f439d9 Roll src/third_party/skia 19304d88c8be..6d927b63a311 (3 commits) (flutter/engine#16782) * effd8a0 Roll src/third_party/skia 6d927b63a311..a6572f78d084 (3 commits) (flutter/engine#16783) * af90e8b Manually add third_party/dart/pkg/stagehand to DEPS (flutter/engine#16785) * ae999f0 Roll fuchsia/sdk/core/mac-amd64 from O6w2L... to 8gjOI... (flutter/engine#16787) * d0897c3 Roll src/third_party/dart 7aa824076c34..2ce1df76309d (11 commits) (flutter/engine#16788) * 7af8d3e Roll src/third_party/skia a6572f78d084..c8d092a060ad (1 commits) (flutter/engine#16789) * fb3dc86 Roll src/third_party/dart 2ce1df76309d..85f6d51c3fd1 (6 commits) (flutter/engine#16792) * 468b371 Roll src/third_party/skia c8d092a060ad..7a6db4cbf48b (2 commits) (flutter/engine#16795) * ff921cd fuchsia: remove use of replace_as_executable (flutter/engine#16690) * d590e98 Evict BitmapCanvas(s) from cache when canvas allocation fails (flutter/engine#16793) * 52070e3 Fix handler unregistration in C++ channels (flutter/engine#16794) * 592b3ff Roll src/third_party/skia 7a6db4cbf48b..d8575452ebf3 (3 commits) (flutter/engine#16799) * 9ac76ad [web] changing user limits for macos (flutter/engine#16797) * 7685e08 [web] Guard the remaining calls to window.onPlatformMessage (flutter/engine#16791) * 3286543 Roll src/third_party/skia d8575452ebf3..adc9bbb2aaca (2 commits) (flutter/engine#16801) * 29cff9e Roll fuchsia/sdk/core/mac-amd64 from 8gjOI... to 3B3a6... (flutter/engine#16803) * fc3a15e Roll src/third_party/skia adc9bbb2aaca..7b96793ccc5b (3 commits) (flutter/engine#16804) * f1a9dc1 Roll src/third_party/skia 7b96793ccc5b..f0a13d04c233 (1 commits) (flutter/engine#16805) * ecabc10 Roll src/third_party/dart 85f6d51c3fd1..418923733006 (30 commits) (flutter/engine#16813)
NoamDev
pushed a commit
to NoamDev/engine
that referenced
this pull request
Feb 27, 2020
MethodChannel and BasicMessageChannel in the C++ wrapper didn't have the expected semantics that passing a null handler would remove any existing handler. This was inconsistent with other platforms and with the lower-level object APIs in this wrapper, and made unregistration in cases where that's desirable more difficult due to needing to keep other object references. Adds tests for this functionality, and some backfill of missing tests for basic handler behavior. See flutter/flutter#51207
NoamDev
added a commit
to NoamDev/engine
that referenced
this pull request
Feb 27, 2020
This reverts commit 4e8406b.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
MethodChannel and BasicMessageChannel in the C++ wrapper didn't have the
expected semantics that passing a null handler would remove any existing
handler. This was inconsistent with other platforms and with the
lower-level object APIs in this wrapper, and made unregistration in
cases where that's desirable more difficult due to needing to keep other
object references.
Adds tests for this functionality, and some backfill of missing tests
for basic handler behavior.
See flutter/flutter#51207