Remove duplicated headers from platform/

Remove duplicated headers from the platform/ directory where the implementation file includes the same dependencies as the corresponding primary header. 

BUG=317231
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161712

[lgombos, 2013-11-10 18:59:33]

changes for platform/

[eseidel, 2013-11-10 22:56:30]

I'm not convinced this is worth the churn?

[lgombos, 2013-11-11 00:01:03]

On 2013/11/10 22:56:30, eseidel wrote:

> I'm not convinced this is worth the churn?

Valid concern, I did consider this myself as well.

One approach we sometimes take to minimize the churn is to to add a rule to the
style checker and start enforcing a rule. I personally think going trough that
process is not justified as the cost outweighs the benefit. I think instead it
makes sense to do a bit of house keeping maybe in every 1-2 years.

Also these CLs does not actually introduce the same churn in the revision
history as most other similar style related changes, since the CL only deletes
unnecessary lines and does not modify existing lines. So there will be a few
more commits in the log, but running a blame on a file to dig into the past will
not be impacted by these changes.

The upside of the change is the same as any other dependency-related cleanup. It
gets us closer to only include dependencies in implementation files that are
actually required for the implementation.

[(unused - use chromium), 2013-11-11 00:02:54]

On Sun, Nov 10, 2013 at 4:01 PM, <l.gombos@samsung.com> wrote:

> On 2013/11/10 22:56:30, eseidel wrote:
>
>  I'm not convinced this is worth the churn?
>>
>
> Valid concern, I did consider this myself as well.
>
> One approach we sometimes take to minimize the churn is to to add a rule
> to the
> style checker and start enforcing a rule. I personally think going trough
> that
> process is not justified as the cost outweighs the benefit. I think
> instead it
> makes sense to do a bit of house keeping maybe in every 1-2 years.
>
> Also these CLs does not actually introduce the same churn in the revision
> history as most other similar style related changes, since the CL only
> deletes
> unnecessary lines and does not modify existing lines. So there will be a
> few
> more commits in the log, but running a blame on a file to dig into the
> past will
> not be impacted by these changes.
>
> The upside of the change is the same as any other dependency-related
> cleanup. It
> gets us closer to only include dependencies in implementation files that
> are
> actually required for the implementation.
>

But maybe the include in the header isn't correct, and the include is only
needed in the cpp file. (Build perf wise, removing headers from cc files
that are already included in the h file won't help at all, since the
compiler will recognize the header guard when including the file the first
time)


>
>
> https://codereview.chromium.org/64643010/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[lgombos, 2013-11-11 00:28:10]

On 2013/11/11 00:02:54, thakis wrote:


> But maybe the include in the header isn't correct, and the include is only
> needed in the cpp file. 

Correct. 

I do not know how to address/enforce the header problem - but I agree that is
more important. 

In a few cases maybe the dependency not needed neither in the header nor in the
cpp - at least this change removes the cpp dependency to make it easier to check
if the dependency is really needed or not.

> (Build perf wise, removing headers from cc files
> that are already included in the h file won't help at all, since the
> compiler will recognize the header guard when including the file the first
> time)

I agree. This effort will likely not improve build time significantly.  The
primary driver was code readability/maintenance.

[lgombos, 2013-11-11 05:56:29]

One more point, just to get it out of my head.

This is an exercise of removing about 800+ lines of dead code, as I consider

#if (always) 0
do something..
#endif

being dead code (no-op) at the moment. I though we intent to remove dead code
and perhaps put it back later if we need it later (exception being upcoming new
features under development).

We could discus if we can have a better heuristic to remove these duplicate
lines from the headers instead - if we do not have that at this time/near
future, than we just continue to carry around this dead code. 

I spot checked ~10 cases and they all looked to me that they should be removed
from the implementation file and not from the header file.

[eseidel, 2013-11-11 06:18:06]

lgtm

I agree this is dead code.  I agree it's OK to remove.  But I also wonder which
of these we'll add back if/when we remove more .h includes from headers.  This
just makes our .cpp includes tighter.  Which is fine.  But doesn't really do
much.

[commit-bot: I haz the power, 2013-11-11 06:18:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/l.gombos@samsung.com/64643010/1

[commit-bot: I haz the power, 2013-11-11 09:35:26]

Change committed as 161712