@mdebbar ptal, I think you may have added this test a few years ago. I'm not sure how we want to handle this, but this test relies on not well specified behavior of JS interop. In particular, JS interop APIs don't have the same type checking guarantees that Dart APIs do, and as we iterate on the new JS interop, these guarantees will be weakened further.

I'm not sure the best path forward. I think the best 'type checking' we can do is ensure that when we get this property, it has a typeof === 'function'.

Would it work if we added type-checking here? :

I'm not sure the best path forward. I think the best 'type checking' we can do is ensure that when we get this property, it has a typeof === 'function'.

We are not type-checking the function here, we are type-checking the arguments passed to the function. We need to make sure the argument is a JsUrlStrategy, but I'm not sure how to do that in the static js interop world. Maybe check that some properties exist on the object? e.g. js_util.hasProperty(obj, 'addPopStateListener')

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Good call, I added a check for addPopStateListener in the non-null case. ptal.