Comment 94 for bug 294712

Revision history for this message
In , Bzbarsky (bzbarsky) wrote :

>+ if (!aNewDecision)
>+ aNewDecision = nsIContentPolicy::ACCEPT;

Still not sure what this is trying to do, exactly...

>+ return NS_OK; // xxx: Should we override future policies?

Imo, yes. Especially since it leads to simpler code.

>+ *aReturn = FindPermission(aURI, aContentType) != -1;

NO_INDEX?

> + inline void CutWWWFromHost(nsCString& host)

I'm confused. None of the existing code is using this function. How do you know it does what the existing code does?

>+ printf("%p: init constructor called [mHost: %s, mContentType: %d, mPolicy: %d]\n", this, mHost.get(), mContentType, mPolicy);

Take those out?

> This method only accepts uris with valid hosts.

That doesn't seem to be correct to me.

>+ * Corresponds to nsIContePolicy's TYPE_*.

nsIContentPolicy (with the "nt" after "e").

I'm still not happy with the "if null" business. You can't have a "null" integer. Just document that behavior is undefined if the value is not a valid ShouldLoad value?

> have a valid host this method does not throw.

How about "does nothing"?

> + * Corresponds to nsIContePolicy's TYPE_*.

Add "nt", as above.

> + * Corresponds to nsIContePolicy's TYPE_*.

And here.