Use __func__ instead of __FUNCTION__.

Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none

Committed: https://crrev.com/f5279485c5c9319fefc51fab2ff26b4672310cc1
Cr-Commit-Position: refs/heads/master@{#408014}

[Peter Kasting, 2016-07-20 02:10:40]

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

[commit-bot: I haz the power, 2016-07-20 02:11:04]

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

[Peter Kasting, 2016-07-20 02:17:20]

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

[commit-bot: I haz the power, 2016-07-20 02:17:42]

Dry run: 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, 2016-07-20 04:18:47]

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

[commit-bot: I haz the power, 2016-07-20 04:18:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peter Kasting, 2016-07-20 05:58:20]

pkasting@chromium.org changed reviewers:
+ esprehn@chromium.org, jbroman@chromium.org, jochen@chromium.org,
liberato@chromium.org, thestig@chromium.org, watk@chromium.org

[Peter Kasting, 2016-07-20 05:58:21]

jbroman: styleguide/ change, per your last message on the relevant thread
thestig: base/ and chrome/ OWNERS
jochen: components/ and content/ OWNERS
watk: media/ OWNERS
liberato: net/ OWNERS
esprehn: third_party/WebKit/Source/* OWNERS

[Peter Kasting, 2016-07-20 05:59:07]

Description was changed from

==========
Use __func__ instead of __FUNCTION__.

BUG=none
TEST=none
==========

to

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none
==========

[esprehn, 2016-07-20 06:12:02]

You can just leave off empty bug and test lines, no reason to use =none.

I do wonder if a bunch of those usages in blink belong. It seems like the media
code is littered with log stuff. I wonder if we should be using something better
to expose what the media stack is doing.

Lgtm w/ one question about DVLOG.

https://codereview.chromium.org/2161193003/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
(right):

https://codereview.chromium.org/2161193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:254:
DVLOG(3) << __func__;
Should this be pretty function? I  guess DVLog has the file name and line number
already?

[Peter Kasting, 2016-07-20 06:16:39]

On 2016/07/20 06:12:02, esprehn wrote:
> You can just leave off empty bug and test lines, no reason to use =none.

It's a good way of saying "I didn't forget to add these, they're just not
relevant".  That's a bit of a bikeshed though (I recall at least one long debate
thread about it...)

> I do wonder if a bunch of those usages in blink belong. It seems like the
media
> code is littered with log stuff. I wonder if we should be using something
better
> to expose what the media stack is doing.

Honestly I would prefer to get rid of nearly all of this stuff everywhere -- I
feel like most logging code isn't necessary and hasn't been used in a while, and
just clutters the source base -- but I'm not in a position to evaluate that for
most of Chromium.

https://codereview.chromium.org/2161193003/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp
(right):

https://codereview.chromium.org/2161193003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/encryptedmedia/NavigatorRequestMediaKeySystemAccess.cpp:254:
DVLOG(3) << __func__;
On 2016/07/20 06:12:02, esprehn wrote:
> Should this be pretty function? I  guess DVLog has the file name and line
number
> already?

Yes, DVLOG will already print the file name and line number.  So probably
__func__ versus __PRETTY_FUNCTION__ isn't too important.

[grt (UTC plus 2), 2016-07-20 06:18:52]

drive-by

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
File chrome/installer/util/delete_after_reboot_helper.cc (right):

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
chrome/installer/util/delete_after_reboot_helper.cc:253: DWORD last_error =
::GetLastError();
while you're at it, could you remove this line and change the one below to:
  DPLOG_IF(...) << __func__;
this will result in a human-readable error message in the log

[Peter Kasting, 2016-07-20 06:27:22]

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
File chrome/installer/util/delete_after_reboot_helper.cc (right):

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
chrome/installer/util/delete_after_reboot_helper.cc:253: DWORD last_error =
::GetLastError();
On 2016/07/20 06:18:51, grt (UTC plus 2) wrote:
> while you're at it, could you remove this line and change the one below to:
>   DPLOG_IF(...) << __func__;
> this will result in a human-readable error message in the log

That last error value is used in the condition for the DLOG.  Does calling
GetLastError() reset the error code?  If so, the change you suggest wouldn't be
safe.  Otherwise I suppose we could do:

  DPLOG_IF(WARNING, length == 0 && GetLastError() != ERROR_PATH_NOT_FOUND)
      << __func__;

(Although personally I wish this would do DVPLOG_IF(1, ...) instead of using
DPLOG().)

[grt (UTC plus 2), 2016-07-20 06:32:47]

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
File chrome/installer/util/delete_after_reboot_helper.cc (right):

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
chrome/installer/util/delete_after_reboot_helper.cc:253: DWORD last_error =
::GetLastError();
On 2016/07/20 06:27:22, Peter Kasting wrote:
> On 2016/07/20 06:18:51, grt (UTC plus 2) wrote:
> > while you're at it, could you remove this line and change the one below to:
> >   DPLOG_IF(...) << __func__;
> > this will result in a human-readable error message in the log
> 
> That last error value is used in the condition for the DLOG.

Ah, I just saw "DLOG" and "gle" in the message without looking too closely.
Apologies.

> Does calling
> GetLastError() reset the error code?

No.

> If so, the change you suggest wouldn't be
> safe.  Otherwise I suppose we could do:
> 
>   DPLOG_IF(WARNING, length == 0 && GetLastError() != ERROR_PATH_NOT_FOUND)
>       << __func__;

I think that's safe. Feel free to leave this out if you prefer not to make a
behavior change with this substitution.

[Peter Kasting, 2016-07-20 06:35:51]

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
File chrome/installer/util/delete_after_reboot_helper.cc (right):

https://codereview.chromium.org/2161193003/diff/20001/chrome/installer/util/d...
chrome/installer/util/delete_after_reboot_helper.cc:253: DWORD last_error =
::GetLastError();
On 2016/07/20 06:32:47, grt (UTC plus 2) wrote:
> On 2016/07/20 06:27:22, Peter Kasting wrote:
> > On 2016/07/20 06:18:51, grt (UTC plus 2) wrote:
> > > while you're at it, could you remove this line and change the one below
to:
> > >   DPLOG_IF(...) << __func__;
> > > this will result in a human-readable error message in the log
> > 
> > That last error value is used in the condition for the DLOG.
> 
> Ah, I just saw "DLOG" and "gle" in the message without looking too closely.
> Apologies.
> 
> > Does calling
> > GetLastError() reset the error code?
> 
> No.
> 
> > If so, the change you suggest wouldn't be
> > safe.  Otherwise I suppose we could do:
> > 
> >   DPLOG_IF(WARNING, length == 0 && GetLastError() != ERROR_PATH_NOT_FOUND)
> >       << __func__;
> 
> I think that's safe. Feel free to leave this out if you prefer not to make a
> behavior change with this substitution.

I don't mind at all, it's more convenient than doing it in a separate CL.

[jbroman, 2016-07-20 12:04:39]

styleguide lgtm

[liberato (no reviews please), 2016-07-20 16:37:35]

On 2016/07/20 05:58:21, Peter Kasting wrote:
> liberato: net/ OWNERS

sorry, but i'm not an owner of net/ -- removing myself.

[liberato (no reviews please), 2016-07-20 16:37:49]

liberato@chromium.org changed reviewers:
- liberato@chromium.org

[Peter Kasting, 2016-07-20 18:36:08]

pkasting@chromium.org changed reviewers:
+ mef@chromium.org

[Peter Kasting, 2016-07-20 18:36:29]

On 2016/07/20 16:37:35, liberato wrote:
> On 2016/07/20 05:58:21, Peter Kasting wrote:
> > liberato: net/ OWNERS
> 
> sorry, but i'm not an owner of net/ -- removing myself.

Oops, I was looking at the wrong OWNERS list.

-> mef for net/ OWNERS

[watk, 2016-07-20 18:41:41]

watk@chromium.org changed reviewers:
- mef@chromium.org

[watk, 2016-07-20 18:41:42]

media/ lgtm

nit: the commit message could be wrapped

[Peter Kasting, 2016-07-20 18:44:19]

Description was changed from

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none
==========

to

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none
==========

[Lei Zhang, 2016-07-20 18:44:24]

lgtm

[watk, 2016-07-20 19:41:11]

watk@chromium.org changed reviewers:
+ mef@chromium.org

[watk, 2016-07-20 19:41:12]

Apparently my comment removed mef@ from the reviewers list.. race condition?
re-adding

[mef, 2016-07-20 19:59:56]

net/ lgtm

[jochen (gone - plz use gerrit), 2016-07-21 08:28:30]

lgtm

[Peter Kasting, 2016-07-21 19:01:30]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2016-07-21 19:01:31]

The patchset sent to the CQ was uploaded after l-g-t-m from esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2161193003/#ps40001
(title: "DPLOG")

[commit-bot: I haz the power, 2016-07-21 19:02:38]

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, 2016-07-21 19:45:15]

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

[commit-bot: I haz the power, 2016-07-21 19:45:18]

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...)

[Peter Kasting, 2016-07-21 21:16:57]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-07-21 21:17:38]

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, 2016-07-22 02:22:49]

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

[commit-bot: I haz the power, 2016-07-22 02:22:52]

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

[Peter Kasting, 2016-07-22 02:43:56]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-07-22 02:44:34]

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, 2016-07-22 04:26:29]

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

[commit-bot: I haz the power, 2016-07-22 04:26:32]

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

[Peter Kasting, 2016-07-22 08:18:25]

Apparently linux_android_rel_ng is not flaky, it's really failing, and the cuase
is that the compiler on that platform violates the standard and expands __func__
to the prettified function name (i.e. including full argument signature) instead
of just the function name.  This causes a variety of log parsing tests to fail.

I can try to hack up the parsing functions to fix this, but it seems like the
compiler should be made to work correctly.  Not sure what compiler that platform
is using.

[Lei Zhang, 2016-07-22 08:54:56]

On 2016/07/22 08:18:25, Peter Kasting wrote:
> I can try to hack up the parsing functions to fix this, but it seems like the
> compiler should be made to work correctly.  Not sure what compiler that
platform
> is using.

g++ 4.9 locally for me with an arm64 build.
https://developer.android.com/ndk/guides/standalone_toolchain.html has a list of
possible toolchains 1/3 of the way down.

[Primiano Tucci (use gerrit), 2016-07-25 16:31:27]

On 2016/07/22 08:54:56, Lei Zhang wrote:
> On 2016/07/22 08:18:25, Peter Kasting wrote:
> > I can try to hack up the parsing functions to fix this, but it seems like
the
> > compiler should be made to work correctly.  Not sure what compiler that
> platform
> > is using.

Sadly this is a bug in the Android NDK, which is defining __func__ =
__PRETTY_FUNCTION__ instead of leaving that as a builtin. See internal bug
b/30353757 I just filed.
Essentially this means that, regardless of the compiler, depending on the fact
that a translation unit ends up including sys/cdefs.h or it will get either the
right (__FUNCTION__) or the wrong (__PRETTY_FUNCTION__) behavior.
It's a sad world, let's go home :(

[Peter Kasting, 2016-07-26 21:48:25]

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

[commit-bot: I haz the power, 2016-07-26 21:48:50]

Dry run: 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, 2016-07-27 01:26:32]

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

[commit-bot: I haz the power, 2016-07-27 01:26:33]

Dry run: This issue passed the CQ dry run.

[Peter Kasting, 2016-07-27 01:51:42]

The CQ bit was checked by pkasting@chromium.org

[Peter Kasting, 2016-07-27 01:51:43]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
thestig@chromium.org, esprehn@chromium.org, jbroman@chromium.org,
watk@chromium.org, mef@chromium.org
Link to the patchset: https://codereview.chromium.org/2161193003/#ps60001
(title: "Resync")

[commit-bot: I haz the power, 2016-07-27 01:52:04]

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, 2016-07-27 02:18:39]

Description was changed from

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none
==========

to

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none
==========

[commit-bot: I haz the power, 2016-07-27 02:18:42]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-27 02:20:39]

Description was changed from

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none
==========

to

==========
Use __func__ instead of __FUNCTION__.

Most of the formatting changes here are due to git cl format, sometimes after
manually collapsing a few string constants.

BUG=none
TEST=none

Committed: https://crrev.com/f5279485c5c9319fefc51fab2ff26b4672310cc1
Cr-Commit-Position: refs/heads/master@{#408014}
==========

[commit-bot: I haz the power, 2016-07-27 02:20:42]

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