Clean up most of the ChromeOS headers in non-ChromeOS build

Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774, 720159
Review-Url: https://codereview.chromium.org/2860553002
Cr-Commit-Position: refs/heads/master@{#470516}
Committed: https://chromium.googlesource.com/chromium/src/+/e120c2fd1e0c07b721caddef640aabf835d75840

[wychen, 2017-05-02 20:36:00]

The CQ bit was checked by wychen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-02 20:36:21]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[wychen, 2017-05-02 20:44:54]

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

[wychen, 2017-05-02 20:44:55]

PTAL

[Nico, 2017-05-02 20:49:22]

lgtm

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/certificate_...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/certificate_...
chrome/browser/certificate_manager_model.cc:27: // TODO: should be guarded by
#if defined(OS_CHROMEOS)
TODOs should be annotated with a name: TODO(wychen) (doesn't mean you have to do
it, just means you're a good point of contact)

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/settings_private/settings_private_event_router.h
(right):

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/settings_private/settings_private_event_router.h:17:
// TODO: should be guarded by #if defined(OS_CHROMEOS)
likewise

[wychen, 2017-05-02 21:57:24]

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/certificate_...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/certificate_...
chrome/browser/certificate_manager_model.cc:27: // TODO: should be guarded by
#if defined(OS_CHROMEOS)
On 2017/05/02 20:49:21, Nico wrote:
> TODOs should be annotated with a name: TODO(wychen) (doesn't mean you have to
do
> it, just means you're a good point of contact)

Done.

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/settings_private/settings_private_event_router.h
(right):

https://codereview.chromium.org/2860553002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/settings_private/settings_private_event_router.h:17:
// TODO: should be guarded by #if defined(OS_CHROMEOS)
On 2017/05/02 20:49:21, Nico wrote:
> likewise

Done.

[wychen, 2017-05-02 23:29:32]

wychen@chromium.org changed reviewers:
+ brettw@chromium.org

[wychen, 2017-05-02 23:29:32]

brettw@chromium.org: Can you have an overall look? Thanks!

[wychen, 2017-05-02 23:34:30]

Description was changed from

==========
Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774
==========

to

==========
Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774
==========

[wychen, 2017-05-02 23:34:30]

wychen@chromium.org changed reviewers:
+ dbeam@chromium.org

[wychen, 2017-05-02 23:35:44]

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

[wychen, 2017-05-02 23:36:52]

dbeam, tfarina, do you think you could take these TODOs?

[tfarina, 2017-05-04 00:22:24]

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
chrome/browser/certificate_manager_model.cc:27: // TODO(wychen): should be
guarded by #if defined(OS_CHROMEOS)
Why can't you ifdef right now?

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/settings_private/settings_private_event_router.h
(right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/settings_private/settings_private_event_router.h:17:
// TODO(wychen): should be guarded by #if defined(OS_CHROMEOS)
Is it a large change if you do it right now?

[wychen, 2017-05-04 01:19:27]

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
chrome/browser/certificate_manager_model.cc:27: // TODO(wychen): should be
guarded by #if defined(OS_CHROMEOS)
On 2017/05/04 00:22:24, tfarina wrote:
> Why can't you ifdef right now?

The types in these chromeos headers are actually used.
It would take another (larger) CL to clean them up.

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/settings_private/settings_private_event_router.h
(right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/settings_private/settings_private_event_router.h:17:
// TODO(wychen): should be guarded by #if defined(OS_CHROMEOS)
On 2017/05/04 00:22:24, tfarina wrote:
> Is it a large change if you do it right now?

Yes. Similar to the case above, the chromeos:: type usage is not guarded by
ifdefs.

[wychen, 2017-05-08 20:06:41]

My plan is to land this first, and then see how these two TODOs can be addressed
in the following CLs. Sounds good?

[Nico, 2017-05-08 20:08:54]

sgtm

[Dan Beam, 2017-05-08 20:34:01]

On 2017/05/08 20:06:41, wychen wrote:
> My plan is to land this first, and then see how these two TODOs can be
addressed
> in the following CLs. Sounds good?

did you ask me because of
chrome/browser/extensions/api/settings_private/settings_private_event_router.h? 
if so, yes, i can definitely handle that

[wychen, 2017-05-08 20:37:09]

On 2017/05/08 20:34:01, Dan Beam wrote:
> On 2017/05/08 20:06:41, wychen wrote:
> > My plan is to land this first, and then see how these two TODOs can be
> addressed
> > in the following CLs. Sounds good?
> 
> did you ask me because of
>
chrome/browser/extensions/api/settings_private/settings_private_event_router.h? 
> if so, yes, i can definitely handle that

That's awesome! Thanks!

[tfarina, 2017-05-09 20:59:41]

On 2017/05/08 20:06:41, wychen wrote:
> My plan is to land this first, and then see how these two TODOs can be
addressed
> in the following CLs. Sounds good?

sgtm

[brettw, 2017-05-09 22:55:25]

lgtm

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
chrome/browser/certificate_manager_model.cc:27: // TODO(wychen): should be
guarded by #if defined(OS_CHROMEOS)
On 2017/05/04 01:19:27, wychen wrote:
> On 2017/05/04 00:22:24, tfarina wrote:
> > Why can't you ifdef right now?
> 
> The types in these chromeos headers are actually used.
> It would take another (larger) CL to clean them up.

Can you clarify this in the comment then? This comment seems confusing in the
current context.

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/settings_private/settings_private_event_router.h
(right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/settings_private/settings_private_event_router.h:17:
// TODO(wychen): should be guarded by #if defined(OS_CHROMEOS)
On 2017/05/04 01:19:27, wychen wrote:
> On 2017/05/04 00:22:24, tfarina wrote:
> > Is it a large change if you do it right now?
> 
> Yes. Similar to the case above, the chromeos:: type usage is not guarded by
> ifdefs.

Ditto with the comment.

For these TODOs, referecing a bug with more information would be helpful.

[wychen, 2017-05-09 23:49:11]

Description was changed from

==========
Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774
==========

to

==========
Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774, 720159
==========

[wychen, 2017-05-10 00:08:14]

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
File chrome/browser/certificate_manager_model.cc (right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/certific...
chrome/browser/certificate_manager_model.cc:27: // TODO(wychen): should be
guarded by #if defined(OS_CHROMEOS)
On 2017/05/09 22:55:25, brettw (behind--catching up) wrote:
> On 2017/05/04 01:19:27, wychen wrote:
> > On 2017/05/04 00:22:24, tfarina wrote:
> > > Why can't you ifdef right now?
> > 
> > The types in these chromeos headers are actually used.
> > It would take another (larger) CL to clean them up.
> 
> Can you clarify this in the comment then? This comment seems confusing in the
> current context.

Done.

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
File
chrome/browser/extensions/api/settings_private/settings_private_event_router.h
(right):

https://codereview.chromium.org/2860553002/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/api/settings_private/settings_private_event_router.h:17:
// TODO(wychen): should be guarded by #if defined(OS_CHROMEOS)
On 2017/05/09 22:55:25, brettw (behind--catching up) wrote:
> On 2017/05/04 01:19:27, wychen wrote:
> > On 2017/05/04 00:22:24, tfarina wrote:
> > > Is it a large change if you do it right now?
> > 
> > Yes. Similar to the case above, the chromeos:: type usage is not guarded by
> > ifdefs.
> 
> Ditto with the comment.
> 
> For these TODOs, referecing a bug with more information would be helpful.

Done.

[wychen, 2017-05-10 00:09:06]

The CQ bit was checked by wychen@chromium.org

[wychen, 2017-05-10 00:09:06]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2860553002/#ps40001
(title: "revise TODO comments")

[commit-bot: I haz the power, 2017-05-10 00:10:45]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 00:17:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 00:17:40]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[wychen, 2017-05-10 00:27:24]

The CQ bit was checked by wychen@chromium.org

[wychen, 2017-05-10 00:27:25]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2860553002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2017-05-10 00:28:19]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 01:23:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-10 01:23:36]

Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[wychen, 2017-05-10 06:49:30]

The CQ bit was checked by wychen@chromium.org

[commit-bot: I haz the power, 2017-05-10 06:50:22]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-10 07:37:16]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494398970905760,
"parent_rev": "a0202037a8eb837583119cdc388c1f04a6c4ab51", "commit_rev":
"e120c2fd1e0c07b721caddef640aabf835d75840"}

[commit-bot: I haz the power, 2017-05-10 07:37:27]

Description was changed from

==========
Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774, 720159
==========

to

==========
Clean up most of the ChromeOS headers in non-ChromeOS build

I ran check_gn_headers.py on linux and android full builds, merged the
missing header file list, and manually fixed ChromeOS-related ones.

Decreased the number of missing header files by 13.
Android build is now free of ChromeOS headers.

BUG=661774, 720159

Review-Url: https://codereview.chromium.org/2860553002
Cr-Commit-Position: refs/heads/master@{#470516}
Committed:
https://chromium.googlesource.com/chromium/src/+/e120c2fd1e0c07b721caddef640a...
==========

[commit-bot: I haz the power, 2017-05-10 07:37:29]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/e120c2fd1e0c07b721caddef640a...