Clipboard hasStrings method on iOS

[justinmc]

Description

hasStrings is needed on iOS to allow us to check for clipboard contents without triggering the new iOS14 clipboard access notification.

Related Issues

flutter/flutter#60145

Tests

I'm desperately struggling to test this, as no other platform channel handlers seem to be tested at all for reference. See the TODOs in my code.

• I was able to call and receive a result from hasStrings via handleMethodCall in a new test file.

[justinmc]

Reviewers: Any idea how to test this? I made some attempts that didn't work and left them commented out with some questions. Let me know if you have any ideas.

Edit: I also tried creating a new FlutterPlatformPluginTest.mm file, but I couldn't get run_tests.py to pick it up.

[justinmc]

Another question for reviewers:

How do we usually expose APIs that only exist on certain platforms (in this case iOS)? Is it alright that the framework needs to know to only call this for iOS devices? Another option is to implement this for the other platforms as well, but re-implement the behavior of hasStrings for them by reading the full clipboard text and returning whether or not it's a pasteable string. Then the framework could call it for any platform.

[GaryQian]

The implementation LGTM, @jmagman for guidance on iOS testing (not too familiar myself)

[GaryQian]

There should be a way to create a mock clipboard, and pass it into the mock engine/plugin, though not clear on implementation details

[GaryQian]

You might be able to extend the class and expose a public version in the test class?

[gaaclarke]

You should be able to make the channel with the same identifier and send a message to it and wait for a reply. There should be an example of this somewhere in our tests but I can't find one. Did you look at this test for ideas? https://github.com/gaaclarke/engine/blob/linting-warning/shell/platform/darwin/common/framework/Source/FlutterChannelsTest.m

[gaaclarke]

I tried this:

- (void)testClipboard {
  id project = OCMClassMock([FlutterDartProject class]);
  FlutterEngine* engine = [[FlutterEngine alloc] initWithName:@"foobar" project:project];
  FlutterMethodChannel* channel =
      [FlutterMethodChannel methodChannelWithName:@"flutter/platform"
                                  binaryMessenger:engine.binaryMessenger
                                            codec:[FlutterJSONMethodCodec sharedInstance]];
  [engine run];
  XCTestExpectation* didCallReply = [self expectationWithDescription:@"didCallReply"];
  [channel invokeMethod:@"Clipboard.getData"
           arguments:nil
              result:^(id _Nullable result){
                [didCallReply fulfill];
              }];
  [self waitForExpectationsWithTimeout:1.0 handler:nil];
}

But it crashes because it tries to run on a fake project. Channels don't work unless an engine is running and the engine doesn't run unless there is a project and in unit tests the engine doesn't have a project... I'll look into this a bit more this afternoon to see what the best approach will be.

[justinmc]

Thanks I appreciate the help.

[gaaclarke]

Ok, I looked at a few options and this appears to be more complicated than I had hoped. handleMethodCall isn't private so you can just call it directly for the test.

[justinmc]

Ah I forgot I could just call it with square brackets in objective c. That seems to work. Thanks for the help!

[GaryQian]

Another question for reviewers:

How do we usually expose APIs that only exist on certain platforms (in this case iOS)? Is it alright that the framework needs to know to only call this for iOS devices? Another option is to implement this for the other platforms as well, but re-implement the behavior of hasStrings for them by reading the full clipboard text and returning whether or not it's a pasteable string. Then the framework could call it for any platform.

cc @Hixie for advice on this?

[Hixie]

The API might not exist in literal form on other platforms but surely we can implement it ourselves based on the APIs they do expose, no? After all, we're going to want to use the same logic to enable/disable paste on all platforms.

[justinmc]

The behavior may be slightly different on different native platforms, but yes I agree. I'll implement this method for our other platforms as well (in separate PRs I think).

[LongCatIsLooong]

I also tried creating a new FlutterPlatformPluginTest.mm file, but I couldn't get run_tests.py to pick it up.

according to the README file:

When you add a new unit test file, also add a reference to that file in shell/platform/darwin/ios/BUILD.gn, under the sources list of the ios_flutter_test target. Once it's there, it will execute with the other tests.

[justinmc]

Thanks for the pointer @LongCatIsLooong! It's working in its own file now.

[justinmc]

@gaaclarke @GaryQian @LongCatIsLooong Ready for a final review.

[LongCatIsLooong]

The implementation LGTM. Is the idea optionally sending the "hasStrings" message based on what defaultTargetPlatform says in the framework, instead of implementing such API for every platform?

[LongCatIsLooong]

Is the generalPasteboard always guaranteed to hasStrings ?

[justinmc]

That's a good point, I'm not sure. I'm going to add a call to setClipboardData right before to guarantee that it's true.

[LongCatIsLooong]

nit: is the arguments supposed to be nil instead?

[justinmc]

Yes, thanks.

[GaryQian]

LGTM!

[justinmc]

@LongCatIsLooong Yes that's right. I plan to implement hasStrings on all platforms and have the framework call it without caring what device it's running on. The alternative would be to implement it only on iOS and have the framework only call it for iOS and use other things like getData for other platforms.

[zanderso]

The Fuchsia test failure is a flake. Landing on red to kick the tree.