Include AIDL files in the presubmit check for IPC security.

Include more file name patterns in the presubmit check for IPC security.

Additionally, rename some files to adhere to the expected naming conventions.

Review-Url: https://codereview.chromium.org/2642973006
Cr-Commit-Position: refs/heads/master@{#445636}
Committed: https://chromium.googlesource.com/chromium/src/+/b19a093aa85d3c3c7babb9ebe8e7342de33a2f74

[palmer, 2017-01-19 22:13:04]

palmer@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2017-01-19 22:15:04]

lgtm

https://codereview.chromium.org/2642973006/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://codereview.chromium.org/2642973006/diff/1/PRESUBMIT.py#newcode1465
PRESUBMIT.py:1465: '*.aidl',
Nit: I probably should have done this, but mind grouping this into sections and
commenting? i.e.

# Legacy IPC
'*_messages.cc',
'*_messages*.h',
...
# Mojo IPC
'*.mojom',
...
# Android <something>
'*.aidl',

[palmer, 2017-01-19 22:35:18]

What about *.typemap, and *_enum_traits*.*? Should we add those?

[dcheng, 2017-01-19 22:37:47]

On 2017/01/19 22:35:18, palmer wrote:
> What about *.typemap, and *_enum_traits*.*? Should we add those?

I've been asking people to name everything struct_traits (even if only holds
enum traits). We can either fix the few violations (my preference) or encourage
the existing convention. WDYT?

[palmer, 2017-01-19 22:42:48]

On 2017/01/19 at 22:37:47, dcheng wrote:
> On 2017/01/19 22:35:18, palmer wrote:
> > What about *.typemap, and *_enum_traits*.*? Should we add those?
> 
> I've been asking people to name everything struct_traits (even if only holds
enum traits). We can either fix the few violations (my preference) or encourage
the existing convention. WDYT?

Well, I also just found 1 *_type_traits*.* file (and then some more in
third_party/, build/, and native_client/). And in any case, it might be hard to
get everyone to adhere to a single struct_traits naming scheme, even if we fix
the few violations now.

We could just say *_traits*.* ? Or would that be overbroad?

[dcheng, 2017-01-19 22:48:10]

On 2017/01/19 22:42:48, palmer wrote:
> On 2017/01/19 at 22:37:47, dcheng wrote:
> > On 2017/01/19 22:35:18, palmer wrote:
> > > What about *.typemap, and *_enum_traits*.*? Should we add those?
> > 
> > I've been asking people to name everything struct_traits (even if only holds
> enum traits). We can either fix the few violations (my preference) or
encourage
> the existing convention. WDYT?
> 
> Well, I also just found 1 *_type_traits*.* file (and then some more in
> third_party/, build/, and native_client/). And in any case, it might be hard
to
> get everyone to adhere to a single struct_traits naming scheme, even if we fix
> the few violations now.
> 
> We could just say *_traits*.* ? Or would that be overbroad?

I think we should fix the type_traits violation (which ironically is for legacy
IPC stuff) -- type_traits is already taken for things that want to do clever
template metaprogramming. Similarly, I think a blanket rule for *_traits*.*
would be too broad.

Largely, it seems like people are (hopefully) following convention and at least
it's relatively easy to find things that don't (I hope), so hopefully we can
keep things in reasonable shape?

[palmer, 2017-01-19 22:48:58]

Description was changed from

==========
Include AIDL files in the presubmit check for IPC security.

AIDL files also need IPC security review. Include them in the check.

BUG=
==========

to

==========
Include more file name patterns in the presubmit check for IPC security.
==========

[palmer, 2017-01-20 22:57:28]

palmer@chromium.org changed reviewers:
+ dpranke@chromium.org, jsbell@chromium.org, sky@chromium.org

[palmer, 2017-01-20 22:58:16]

Description was changed from

==========
Include more file name patterns in the presubmit check for IPC security.
==========

to

==========
Include more file name patterns in the presubmit check for IPC security.

Additionally, rename some files to adhere to the expected naming conventions.
==========

[palmer, 2017-01-20 22:58:49]

This should be a mechanical change, and relatively easy to review. PTAL. Thanks!

[palmer, 2017-01-20 22:58:56]

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

[commit-bot: I haz the power, 2017-01-20 22:59:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jsbell, 2017-01-20 23:12:29]

mechanically lgtm, one nit...

https://codereview.chromium.org/2642973006/diff/80001/content/common/indexed_...
File content/common/indexed_db/indexed_db_struct_traits.cc (right):

https://codereview.chromium.org/2642973006/diff/80001/content/common/indexed_...
content/common/indexed_db/indexed_db_struct_traits.cc:240: using
indexed_db::mojom::CursorDirection;
It's a little unusual to see these usings way down in the middle of the file.
They're almost invariably all grouped at the top of the file.

[jsbell, 2017-01-20 23:13:32]

previous is for just the content/common/indexed_db directory, of course

[commit-bot: I haz the power, 2017-01-20 23:54:07]

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

[commit-bot: I haz the power, 2017-01-20 23:54:08]

Dry run: This issue passed the CQ dry run.

[palmer, 2017-01-21 00:02:17]

https://codereview.chromium.org/2642973006/diff/80001/content/common/indexed_...
File content/common/indexed_db/indexed_db_struct_traits.cc (right):

https://codereview.chromium.org/2642973006/diff/80001/content/common/indexed_...
content/common/indexed_db/indexed_db_struct_traits.cc:240: using
indexed_db::mojom::CursorDirection;
> It's a little unusual to see these usings way down in the middle of the file.
They're almost invariably all grouped at the top of the file.

Done.

[sky, 2017-01-21 00:31:26]

LGTM

[Dirk Pranke, 2017-01-24 02:32:06]

PRESUBMIT.py change lgtm.

[palmer, 2017-01-24 03:02:19]

The CQ bit was checked by palmer@chromium.org

[palmer, 2017-01-24 03:02:19]

The patchset sent to the CQ was uploaded after l-g-t-m from dcheng@chromium.org,
jsbell@chromium.org
Link to the patchset: https://codereview.chromium.org/2642973006/#ps100001
(title: "Move `using` statements.")

[commit-bot: I haz the power, 2017-01-24 03:02:59]

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-01-24 04:00:36]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485226939105410,
"parent_rev": "ca350bd03d0ee72203c44e0f708d04747e68eb27", "commit_rev":
"b19a093aa85d3c3c7babb9ebe8e7342de33a2f74"}

[commit-bot: I haz the power, 2017-01-24 04:01:10]

Description was changed from

==========
Include more file name patterns in the presubmit check for IPC security.

Additionally, rename some files to adhere to the expected naming conventions.
==========

to

==========
Include more file name patterns in the presubmit check for IPC security.

Additionally, rename some files to adhere to the expected naming conventions.

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

[commit-bot: I haz the power, 2017-01-24 04:01:17]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/b19a093aa85d3c3c7babb9ebe8e7...

[Wez, 2017-02-08 02:13:58]

Description was changed from

==========
Include more file name patterns in the presubmit check for IPC security.

Additionally, rename some files to adhere to the expected naming conventions.

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

to

==========
Include more file name patterns in the presubmit check for IPC security.

Additionally, rename some files to adhere to the expected naming conventions.

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

[Wez, 2017-02-08 02:13:58]

wez@chromium.org changed reviewers:
+ jln@chromium.org

[Wez, 2017-02-08 02:22:56]

Hallo jln@chromium.org!
Due to a depot_tools patch which mistakenly removed the OWNERS check for
non-source files (see crbug.com/684270), the following files landed in this CL
and need a retrospective review from you:
	content/common/BUILD.gn
Thanks,
Wez

[jln (very slow on Chromium), 2017-02-10 04:43:16]

On 2017/02/08 02:22:56, Wez wrote:
> Hallo mailto:jln@chromium.org!
> Due to a depot_tools patch which mistakenly removed the OWNERS check for
> non-source files (see crbug.com/684270), the following files landed in this CL
> and need a retrospective review from you:
> 	content/common/BUILD.gn
> Thanks,
> Wez

I only review content/common/* for sandbox changes, but the change is trivial so
LGTM.