Added presubmit warnings about base coordinate system.

Added presubmit warnings about base coordinate system.

BUG=132984
TEST=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=142578

[Avi (use Gerrit), 2012-06-15 18:46:16]

Should it be an error rather than a warning?

[Mark Mentovai, 2012-06-15 18:50:52]

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py#newcode34
PRESUBMIT.py:34: _BANNED_FUNCTIONS = (
I would name this _BANNED_FUNCTIONS_OBJC…

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py#newcode286
PRESUBMIT.py:286: file_filter = lambda f: f.LocalPath().endswith(('.cc', '.mm',
'.h'))
…and omit .cc but include .m.

[Mark Mentovai, 2012-06-15 18:51:16]

Once you’re done fixing things up, how many violations will exist in Chrome code
where the presubmit script would find them?

[Avi (use Gerrit), 2012-06-15 18:55:24]

> Once you’re done fixing things up, how many violations will exist in Chrome
code
> where the presubmit script would find them?

If they were modified?

Re the base calls, there are two legitimate ones that exist; I fixed all the
rest.

Re the tracking area/rect calls, too many to count :(

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py#newcode286
PRESUBMIT.py:286: file_filter = lambda f: f.LocalPath().endswith(('.cc', '.mm',
'.h'))
I did it this way deliberately. I wanted a general mechanism; if other platforms
want to plug into this, they just need to add to the table.

[Mark Mentovai, 2012-06-15 18:58:25]

> Re the tracking area/rect calls, too many to count :(

If [almost] all of those are wrong, then it’s OK to make this a hard error.
Otherwise, it’s best as a warning.

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py
File PRESUBMIT.py (right):

https://chromiumcodereview.appspot.com/10556015/diff/1/PRESUBMIT.py#newcode286
PRESUBMIT.py:286: file_filter = lambda f: f.LocalPath().endswith(('.cc', '.mm',
'.h'))
Avi wrote:
> I did it this way deliberately. I wanted a general mechanism; if other
platforms
> want to plug into this, they just need to add to the table.

Then give ’em another table to plug into? I don’t see the point in checking for
Objective-C-isms in .cc files.

[Avi (use Gerrit), 2012-06-15 19:05:08]

Re the tracking areas, it's hard to say if they're _wrong_. Talk to rsesek about
http://code.google.com/p/chromium/issues/detail?id=48709 . We already banned
their use.

Re the table, do you want me to pre-create it and leave it empty?

[Mark Mentovai, 2012-06-15 19:06:49]

Yeah, exactly, an empty table.

I don’t know, I’m on the warning/error fence, then. Maybe you can add an extra
attribute to your table and have the ones we’re sure we’ve eradicated be errors,
and have the ones we can’t decide about be warnings?

[Avi (use Gerrit), 2012-06-15 19:20:33]

OK, please take a look.

[Mark Mentovai, 2012-06-15 19:22:21]

LGTM

[commit-bot: I haz the power, 2012-06-15 19:24:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10556015/12001

[commit-bot: I haz the power, 2012-06-15 19:42:32]

Try job failure for 10556015-12001 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[Mark Mentovai, 2012-06-15 19:43:09]

Ha, yeah right!

[commit-bot: I haz the power, 2012-06-15 19:49:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10556015/12001

[commit-bot: I haz the power, 2012-06-15 20:06:05]

Try job failure for 10556015-12001 (retry) on android for steps "compile, build"
(clobber build).
It's a second try, previously, steps "compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[commit-bot: I haz the power, 2012-06-15 20:12:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10556015/12001

[commit-bot: I haz the power, 2012-06-15 20:40:35]

Try job failure for 10556015-12001 (retry) on linux_chromeos for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2012-06-15 22:01:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10556015/12001

[commit-bot: I haz the power, 2012-06-15 23:20:52]

Try job failure for 10556015-12001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-06-16 02:22:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/10556015/12001

[commit-bot: I haz the power, 2012-06-16 05:06:01]

Change committed as 142578