Important note for files in content\test

[John Abd-El-Malek]

Hi owners of content\test,

Just a quick note that you should ensure that no internal (i.e. non content\public) headers are leaked through headers in content\test. i.e. we don't want test code in src\chrome\renderer to be able to get to src\content\renderer. This is just like test files in chrome or content can't include files in webcore.

[Paweł Hajdan, Jr.]

Couldn't that be enforced using DEPS files? Maybe by moving content\test headers to content\public (or content\public\test, or \content\test\public) so that content\test implementation still has access to internals?

My point here is that it is easy to miss those things during reviews, and automatic checks work much better.

Anyway, I'll take that into account when doing reviews.

[John Abd-El-Malek]

I was hoping to avoid that because I find it a little confusing when the headers aren't beside the cc files. But you're right, this is the only way to enforce that. I'll move the headers.

[Jói Sigurðsson]

Wait, wouldn't it be easier to update the DEPS tool to understand this
kind of relationship (i.e. a dependency allows from .cc files but not
.h files)? I think moving the headers is confusing and I think this
kind of DEPS feature would be useful in other places.

Cheers,
Jói

[Jói Sigurðsson]

(and I volunteer to help with the DEPS tool change if needed)

[Jói Sigurðsson]

I see that you've already moved the headers, so my comment was a bit
late. I still think the DEPS feature I suggested might be useful e.g.
as I work on modularizing chrome/browser, but unless you want to
switch to that approach I probably won't work on it right away.

Cheers,
Jói

[John Abd-El-Malek]

(hope I didn't jump the gun too much :) I figured the weekend would be good for file moves.)

One issue with just depending on enhancing DEPS is that there are headers in content\test that will remain in content\test, because they're only used internally by content. These include other headers from content.

[Jói Sigurðsson]

> (hope I didn't jump the gun too much :) I figured the weekend would be good
> for file moves.)

Not at all, the changes are fine.

> One issue with just depending on enhancing DEPS is that there are headers in
> content\test that will remain in content\test, because they're only used
> internally by content. These include other headers from content.

Hmm, yeah good point. Why is nothing ever simple? ;-)

Cheers,
Jói