Fix presubmit script for .cc files which include system files first.

Fix presubmit script for .cc files which include system files first.

Some files which need specialized order of includes are excluded in
most of the checks, however when handling the special case first
include file the excluded checks were not being skipped.

R=phajdan.jr@chromium.org
BUG=None
TEST=Tested "git cl presubmit --force" with gpu_channel.cc changes.

[M-A Ruel, 2015-09-01 19:02:59]

I'm not a src/PRESUBMIT.py owner.

[David Yen, 2015-09-01 19:04:56]

dyen@chromium.org changed reviewers:
+ phajdan.jr@chromium.org
- maruel@chromium.org

[David Yen, 2015-09-01 19:04:57]

This bug is causing gpu_channel.cc to not pass the presubmit test, here is what
the file looks like:

https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...

Has this been changed recnetly? I'm not sure how people have been landing
patches otherwise unless they have just been bypassing hooks.

+phajdan.jr for review.

-maruel from review, sorry I thought you were an owner based on the log history.

[Paweł Hajdan Jr., 2015-09-02 10:43:46]

Could you explain more why is this needed?

Why does the .cc file needs to #include windows.h first?

FWIW if you want to make any changes to the presubmit check, make sure there's a
corresponding change to its unit tests.

[David Yen, 2015-09-02 15:58:48]

On 2015/09/02 10:43:46, Paweł Hajdan Jr. wrote:
> Could you explain more why is this needed?
> 
> Why does the .cc file needs to #include windows.h first?
> 
> FWIW if you want to make any changes to the presubmit check, make sure there's
a
> corresponding change to its unit tests.

I can look closer into why this particular case includes files in that order,
but judging from the description in PRESUBMIT.py the include order should be
valid:

"Some files need specialized order of includes; exclude such files from this
check."

I have added a unit test for this as well.

[David Yen, 2015-09-02 17:03:22]

On 2015/09/02 15:58:48, David Yen wrote:
> On 2015/09/02 10:43:46, Paweł Hajdan Jr. wrote:
> > Could you explain more why is this needed?
> > 
> > Why does the .cc file needs to #include windows.h first?
> > 
> > FWIW if you want to make any changes to the presubmit check, make sure
there's
> a
> > corresponding change to its unit tests.
> 
> I can look closer into why this particular case includes files in that order,
> but judging from the description in PRESUBMIT.py the include order should be
> valid:
> 
> "Some files need specialized order of includes; exclude such files from this
> check."
> 
> I have added a unit test for this as well.

After investigating this particular file (gpu_channel.cc), it seems like the
windows include file can be included after so this is not a blocker for me. 

Judging from the description my patch still seems necessary though.

[Paweł Hajdan Jr., 2015-09-03 13:23:52]

https://codereview.chromium.org/1309283003/diff/40001/PRESUBMIT_test.py
File PRESUBMIT_test.py (right):

https://codereview.chromium.org/1309283003/diff/40001/PRESUBMIT_test.py#newco...
PRESUBMIT_test.py:139: contents = ['#include "ipc/some_macros.h"',
This doesn't look like a realistic scenario we'd like to support.

That makes it less suitable for a test.

Do you possibly have a better scenario? Looks like with maybe fixing
gpu_channel.cc no changes would be necessary here. Are any other files in the
tree affected?

[David Yen, 2015-09-03 16:38:52]

On 2015/09/03 13:23:52, Paweł Hajdan Jr. wrote:
> https://codereview.chromium.org/1309283003/diff/40001/PRESUBMIT_test.py
> File PRESUBMIT_test.py (right):
> 
>
https://codereview.chromium.org/1309283003/diff/40001/PRESUBMIT_test.py#newco...
> PRESUBMIT_test.py:139: contents = ['#include "ipc/some_macros.h"',
> This doesn't look like a realistic scenario we'd like to support.
> 
> That makes it less suitable for a test.
> 
> Do you possibly have a better scenario? Looks like with maybe fixing
> gpu_channel.cc no changes would be necessary here. Are any other files in the
> tree affected?

The more realistic scenario is the include order for windows.h, a quick search
showed a couple of examples but I'm not sure if the order could be changed in
those cases:

https://code.google.com/p/chromium/codesearch#chromium/src/base/base_paths_wi...

https://code.google.com/p/chromium/codesearch#chromium/src/chrome_elf/chrome_...

https://code.google.com/p/chromium/codesearch#chromium/src/device/serial/seri...

If you don't think this is necessary then I'm ok with just closing the issue.
Although the other issue then, is the error messages that come up because of
this is completely wrong. Because those headers are skipped in all the other
checks, it currently will tell you the first include after the header include is
in the wrong place.

[David Yen, 2015-09-03 16:39:27]

https://codereview.chromium.org/1309283003/diff/40001/PRESUBMIT_test.py
File PRESUBMIT_test.py (right):

https://codereview.chromium.org/1309283003/diff/40001/PRESUBMIT_test.py#newco...
PRESUBMIT_test.py:139: contents = ['#include "ipc/some_macros.h"',
On 2015/09/03 13:23:52, Paweł Hajdan Jr. wrote:
> This doesn't look like a realistic scenario we'd like to support.
> 
> That makes it less suitable for a test.
> 
> Do you possibly have a better scenario? Looks like with maybe fixing
> gpu_channel.cc no changes would be necessary here. Are any other files in the
> tree affected?

Done. Changed the example to windows.h to be more realistic.