AppDomain APIs about standard HOT 21 CLOSED

I've factored the AppDomain APIs into a separate cs file (https://github.com/dotnet/standard/blob/master/netstandard/ref/mscorlib.appdomain.cs) so they will easier to review (it is impossible to review all of mscorlib.cs on the web side) as we factor out the other API sets out of the standard.

from standard.

@jkotas @rahku @DnlHarvey Can you guys take a look at the latest proposed set of AppDomain APIs at https://github.com/dotnet/standard/blob/master/netstandard/ref/mscorlib.appdomain.cs and let me know if you guys see any other issues.

cc @jfree @gkhanna79

from standard.

AppDomainManager and AppDOmainSetup are only applicable if we create new domains. I don't think it is applicable for defaultdomain.

from standard.

Below two are under FEATURE_CLICKONCE. Should we be including these?
public System.ActivationContext ActivationContext { get { throw null; } }
public System.ApplicationIdentity ApplicationIdentity { get { throw null; } }

from standard.

Following seem related to CAS but are not marked as such
public void SetThreadPrincipal(System.Security.Principal.IPrincipal principal) { }
public void SetPrincipalPolicy(System.Security.Principal.PrincipalPolicy policy) { }

Will these be throwing notSupported?
CreateDomain
public static void Unload(System.AppDomain domain) { }

from standard.

I'm still on the fence about AppDomainManager and AppDomainSetup, right now they don't appear to have any value and are heavily sub-setted. Perhaps they are better not being included in netstandard but instead being an extension with all the APIs in the future if we need them @jkota @terrajobst I'm curious if you guys agree with that.

CreateDomain/Unload yes throw PNSE.

SetThreadPrincipal/SetPrinciplaPolicy - They don't look CAS related to me how do they connect to CAS?

ActivationContext/ApplicationIdentity yes those are ClickOnce but I don't believe they really hurt anything, however with that said I'm going to talk a closer look at the ClickOnce APIs.

from standard.

Perhaps they are better not being included in netstandard

Sounds good to me.

SetThreadPrincipal/SetPrinciplaPolicy

Right, these are not tightly coupled with CAS. They control the default value for Thread.CurrentPrincipal.

ActivationContext/ApplicationIdentity

They are in the same logical bucket as AppDomainManager for me. If you trim AppDomainManager, you can trim these as well.

from standard.

1b3f106
ed2c486

Those two commits should do the final clean-up of the AppDomain APIs. Please take another look at https://github.com/dotnet/standard/blob/master/netstandard/ref/mscorlib.appdomain.cs to be sure the rest seems reasonable and then I will close out this issue.

from standard.

Looks reasonable to me.

from standard.

Regarding FirstChanceExceptionEvent. We have never supported this for coreclr https://github.com/dotnet/coreclr/blob/775003a4c72f0acc37eab84628fcef541533ba4e/src/vm/excep.cpp#L12924.
@gkhanna79 do you happen to remember the reason? I will try removing the ifdef and see if it works.

from standard.

Some of the apis like AppDomain.SetShadowCopyPath are marked as obsolete do we need to port these too?

from standard.

Is there a reason for clubbing Activator type with AppDomain?

from standard.

Otherwise looks fine to me.

from standard.

Some of the apis like AppDomain.SetShadowCopyPath are marked as obsolete do we need to port these too?

We have a lot of obsolete methods in netstandard, in the name of compat. If we cannot support it then then throw PNSE.

from standard.

Is there a reason for clubbing Activator type with AppDomain?

I put it in the same file mainly because it was being sliced and diced similar to AppDomain so it would be easier to review in this file.

from standard.

AppDomainUnloadedException

I dont believe this is applicable since DefaultDomain cannot be unloaded. Unless we are planning to support multiple AppDomains, I think we should not expose this.

from standard.

Unload

The only AppDomain this can be made to work upon is DefaultDomain. And since we cannot unload it, we will throw "CannotUnloadAppDomainException".

Are these both required given we do not have multiple AppDomains?

from standard.

I think we should not expose this.

We are exposing everything by default in name of compatibility and easy migration; unless there is very strong reason not to. There is a ton of types getting exposes that are not useful on its own; but they are very useful to get existing code to compile.

from standard.

Are we simply exposing every type that is not in the set of things we do not want to expose (e.g. CAS, Remoting, etc) or do we consider their usage count as well (i.e. do not expose if the usage is 0%)?

from standard.

Are we simply exposing every type that is not in the set of things we do not want to expose (e.g. CAS, Remoting, etc) or do we consider their usage count as well (i.e. do not expose if the usage is 0%)?

Our default answers is that we are just exposing everything that is in the intersection of the platforms we want to support. Usage data is only one data point which we do consider but it is only a supporting data point as opposed to a deciding one.

from standard.

I think we have closed on the set of APIs here so I'm closing this issue.

from standard.