Won't this break the existing build? I feel like we need to do a manual SDK roll with the change to fake_flatland included.

Nothing uses SetOpacity right now, so commenting it out won't break anything. If it broke the build, CQ would fail. :)

The FIDL base class which FakeFlatland derives from implements every method already with a LOG(FATAL). So this change just means that SetOpacity calls would CHECK -- but nothing calls SetOpacity in flutter right now.

Once the SDK rolls I'll land a 2nd PR to re-enable this - notice I already changed the name and logic in the method, so all I'll need to do is uncomment everything.

I see, I thought the methods in Flatland_TestBase were pure virtual and needed to be linked, instead of stubs. Sorry for the confusion.

Do the C++ bindings have the equivalent of the dart package:fidl_foo/fidl_test.dart files which protect against breakages like this?

Yeah, thats what the Flatland_TestBase is, its the C++ version of the auto-generated FIDL test stubs.

However these FIDL stubs only protect against Fuchsia adding a new method. That way the new method becomes a stub instead of a linker error when we generate the C++ code from it.

In the case of this CL Fuchsia removed the SetOpacity method (well, they renamed it). None of the FIDL stubs in any language protect against that.

It's not their fault, the engineer who renamed it didn't know about this FakeFlatland. That's why it needs to move into the SDK so the Scenic team can maintain it.

I see, I thought the methods in Flatland_TestBase were pure virtual and needed to be linked, instead of stubs. Sorry for the confusion.