Cleanup: Remove unneeded grit include in chrome.

Cleanup: Remove unneeded grit include in chrome.

Also fix up tools/unused-grit-header.py to deal with some more grit formatting and corner cases.

Committed: https://crrev.com/5156fbffe13e2581984850b5654286bee306ac38
Cr-Commit-Position: refs/heads/master@{#292701}

[Lei Zhang, 2014-08-27 01:38:38]

Patchset #3 (id:40001) has been deleted

[Lei Zhang, 2014-08-27 02:24:44]

Patchset #3 (id:60001) has been deleted

[Lei Zhang, 2014-08-27 02:24:49]

Patchset #2 (id:20001) has been deleted

[Lei Zhang, 2014-08-27 02:24:55]

Patchset #1 (id:1) has been deleted

[Lei Zhang, 2014-08-27 18:56:56]

thestig@chromium.org changed reviewers:
+ thakis@chromium.org

[Lei Zhang, 2014-08-29 17:23:57]

PTAL. Maybe I didn't send this out earlier...

[tfarina, 2014-08-29 17:45:45]

tfarina@chromium.org changed reviewers:
+ tfarina@chromium.org

[tfarina, 2014-08-29 17:45:46]

Lei, I was thinking about the naming of this script.

Now I realize, what you think if we rename it to list-unused-grit-includes.py?
That makes (to me) its purpose more easily to be understood (at least imo).

Does it automatically remove the includes for you?

[Lei Zhang, 2014-08-29 17:50:55]

On 2014/08/29 17:45:46, tfarina wrote:
> Lei, I was thinking about the naming of this script.
> 
> Now I realize, what you think if we rename it to list-unused-grit-includes.py?
> That makes (to me) its purpose more easily to be understood (at least imo).

I don't care either way.

> Does it automatically remove the includes for you?

Nope.

[Nico, 2014-08-29 17:54:55]

lgtm

https://codereview.chromium.org/514493002/diff/140001/chrome/third_party/mozi...
File chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp (left):

https://codereview.chromium.org/514493002/diff/140001/chrome/third_party/mozi...
chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp:46: #include
"ui/base/l10n/l10n_util.h"
Hm, I wonder why this even got added, given that this is a 3rd-party file.
Needlessly added in
https://codereview.chromium.org/661241/diff/4001/chrome/third_party/mozilla_s...
, hmm.

[tfarina, 2014-08-29 17:55:55]

https://codereview.chromium.org/514493002/diff/120001/tools/unused-grit-heade...
File tools/unused-grit-header.py (right):

https://codereview.chromium.org/514493002/diff/120001/tools/unused-grit-heade...
tools/unused-grit-header.py:153: 'IMAGE_GRID(',  # Macro in
nine_image_painter_factory.h
https://chromium.googlesource.com/chromium/src/+/master/tools/resources/find_...
has some logic for this particular case.

[Lei Zhang, 2014-08-29 18:00:36]

On 2014/08/29 17:55:55, tfarina wrote:
>
https://codereview.chromium.org/514493002/diff/120001/tools/unused-grit-heade...
> File tools/unused-grit-header.py (right):
> 
>
https://codereview.chromium.org/514493002/diff/120001/tools/unused-grit-heade...
> tools/unused-grit-header.py:153: 'IMAGE_GRID(',  # Macro in
> nine_image_painter_factory.h
>
https://chromium.googlesource.com/chromium/src/+/master/tools/resources/find_...
> has some logic for this particular case.

Thanks. How about I do that + rename in a separate CL after this? I already
started try jobs.

[Nico, 2014-08-29 18:01:49]

sgtm


On Fri, Aug 29, 2014 at 11:00 AM, <thestig@chromium.org> wrote:

> On 2014/08/29 17:55:55, tfarina wrote:
>
> https://codereview.chromium.org/514493002/diff/120001/
> tools/unused-grit-header.py
>
>> File tools/unused-grit-header.py (right):
>>
>
>
> https://codereview.chromium.org/514493002/diff/120001/
> tools/unused-grit-header.py#newcode153
>
>> tools/unused-grit-header.py:153: 'IMAGE_GRID(',  # Macro in
>> nine_image_painter_factory.h
>>
>
> https://chromium.googlesource.com/chromium/src/+/master/
> tools/resources/find_unused_resources.py
>
>> has some logic for this particular case.
>>
>
> Thanks. How about I do that + rename in a separate CL after this? I already
> started try jobs.
>
> https://codereview.chromium.org/514493002/
>

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

[tfarina, 2014-08-29 18:12:20]

On 2014/08/29 18:00:36, Lei Zhang wrote:
> On 2014/08/29 17:55:55, tfarina wrote:
> >
>
https://codereview.chromium.org/514493002/diff/120001/tools/unused-grit-heade...
> > File tools/unused-grit-header.py (right):
> > 
> >
>
https://codereview.chromium.org/514493002/diff/120001/tools/unused-grit-heade...
> > tools/unused-grit-header.py:153: 'IMAGE_GRID(',  # Macro in
> > nine_image_painter_factory.h
> >
>
https://chromium.googlesource.com/chromium/src/+/master/tools/resources/find_...
> > has some logic for this particular case.
> 
> Thanks. How about I do that + rename in a separate CL after this? I already
> started try jobs.

Sure. rslgtm.

[mattm, 2014-08-29 18:57:53]

mattm@chromium.org changed reviewers:
+ mattm@chromium.org

[mattm, 2014-08-29 18:57:54]

https://codereview.chromium.org/514493002/diff/140001/chrome/third_party/mozi...
File chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp (left):

https://codereview.chromium.org/514493002/diff/140001/chrome/third_party/mozi...
chrome/third_party/mozilla_security_manager/nsNSSCertificate.cpp:46: #include
"ui/base/l10n/l10n_util.h"
On 2014/08/29 17:54:55, Nico (hiding) wrote:
> Hm, I wonder why this even got added, given that this is a 3rd-party file.
> Needlessly added in
>
https://codereview.chromium.org/661241/diff/4001/chrome/third_party/mozilla_s...
> , hmm.

These files are under third_party since they are based off of mozilla code, but
they are all adapted for chrome, not just straight imports.

Still doesn't explain why these includes got added though, probably just some
refactoring while writing the CL and didn't clean up properly.

[Lei Zhang, 2014-08-29 18:58:28]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-29 18:59:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/514493002/140001

[Lei Zhang, 2014-08-29 21:41:26]

The CQ bit was unchecked by thestig@chromium.org

[Lei Zhang, 2014-08-29 21:41:33]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2014-08-29 21:59:52]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/thestig@chromium.org/514493002/140001

[commit-bot: I haz the power, 2014-08-29 22:48:49]

Committed patchset #4 (id:140001) as a86cbf4cffc2ece9a6abab4972565eb35e3e8287

[commit-bot: I haz the power, 2014-09-10 03:11:12]

Patchset 4 (id:??) landed as
https://crrev.com/5156fbffe13e2581984850b5654286bee306ac38
Cr-Commit-Position: refs/heads/master@{#292701}