Add support for setting allow http flag in Dart VM #17653
Conversation
| // The SDK expects this field to represent "allow http" so we switch the | ||
| // value. | ||
| Dart_Handle allow_http_value = disable_http ? Dart_False() : Dart_True(); |
There was a problem hiding this comment.
If the SDK uses "allow", why use "disable" here? The inconsistency (especially in C++ where there are not named parameters) increases the likelihood for a potential mismatch.
(The style guide for the flutter/flutter repo discourages negative names. Granted, that's not a style guide for flutter/engine, but I think it's still a good convention to follow.)
There was a problem hiding this comment.
I struggled with this too. The problem is that boolean switches in engine currently don't test for values. They simply infer true if the switch exists and false if it does not. This works well only for settings whose default value is false.
I would ideally have a switch for allow_http and then be able to do --noallow_http from the embedder. My understanding is that that would require a large-ish change to the switch processing (and it is only relevant for Android). I figured it is not worth doing especially with many examples of negative names in engine already.
Another simple way to solve this is to have an explicit switch for --noallow_http. But that doesn't really solve the problem. You still end up with a negative name and the negation is pushed to somewhere else.
mehmetf
added
the
waiting for tree to go green
This is not a breaking change. It simply allows embedding code to set
disable_httpsetting. Related to flutter/flutter#54448.