encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h macros

encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522, 596760
Committed: https://crrev.com/060d0889ac68f2bd629ab3b684723dd32fce7a88
Cr-Commit-Position: refs/heads/master@{#395791}

[Srirama, 2016-05-23 17:05:21]

Description was changed from

==========
fix

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

to

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of
base/logging.h macros in core/html/ folder.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522
==========

[Srirama, 2016-05-23 17:05:53]

Description was changed from

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of
base/logging.h macros in core/html/ folder.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522
==========

to

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522
==========

[Srirama, 2016-05-23 17:06:34]

srirama.m@samsung.com changed reviewers:
+ jrummell@chromium.org

[Srirama, 2016-05-23 17:06:35]

PTAL

[jrummell, 2016-05-23 22:53:56]

Thanks for doing this. Most of the comments apply to all files (e.g. add
base/logging.h to #includes, remove wtf/Assertions.h if there).

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/ContentDecryptionModuleResultPromise.cpp:11:
#include "wtf/Assertions.h"
This should be removed and base/logging.h added.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:22:
#include "wtf/Functional.h"
#include base/logging.h

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:121:
DVLOG(EME_LOG_LEVEL) << "SetMediaKeysHandler";
We should probably use __FUNCTION__ instead of the name.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:318:
DVLOG(EME_LOG_LEVEL) << "setMediaKeys current(" << thisElement.m_mediaKeys.get()
<< "), new(" << mediaKeys << ")";
std::hex and std::dec?

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:149:
DCHECK_EQ(m_type, GenerateRequest);
Typically the expected value is first, the variable second.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:327:
DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << "MediaKeySession(" << this << ")";
To keep the message consistent I think this needs to be "std::hex << this <<
std::dec". Maybe overload operator<< to return "MediaKeySession(xxxx)::" and
then use << this << __FUNCTION__?

[Srirama, 2016-05-24 09:57:59]

Patchset #2 (id:20001) has been deleted

[Srirama, 2016-05-24 10:09:37]

Patchset #2 (id:40001) has been deleted

[Srirama, 2016-05-24 16:24:04]

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:22:
#include "wtf/Functional.h"
On 2016/05/23 22:53:56, jrummell wrote:
> #include base/logging.h

Not able to include base/logging.h because of dependency rules.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:121:
DVLOG(EME_LOG_LEVEL) << "SetMediaKeysHandler";
On 2016/05/23 22:53:56, jrummell wrote:
> We should probably use __FUNCTION__ instead of the name.

Done.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:318:
DVLOG(EME_LOG_LEVEL) << "setMediaKeys current(" << thisElement.m_mediaKeys.get()
<< "), new(" << mediaKeys << ")";
On 2016/05/23 22:53:56, jrummell wrote:
> std::hex and std::dec?

Why std::hex and std::dec? I guess, you want "this" pointer to be printed as hex
address? It is already printing in that format.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:149:
DCHECK_EQ(m_type, GenerateRequest);
On 2016/05/23 22:53:56, jrummell wrote:
> Typically the expected value is first, the variable second.

Done.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:327:
DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << "MediaKeySession(" << this << ")";
On 2016/05/23 22:53:56, jrummell wrote:
> To keep the message consistent I think this needs to be "std::hex << this <<
> std::dec". Maybe overload operator<< to return "MediaKeySession(xxxx)::" and
> then use << this << __FUNCTION__?

DVLOG already prints filename and line number, so do we need "MediaKeySession::"
to be printed? We are just printing function name in HTMLMedia* file and other
media files. So i think __FUNCTION__ is good enough. WDYT?

[jrummell, 2016-05-24 18:07:11]

Can you add 596760 to the bug list? After that, LGTM.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:22:
#include "wtf/Functional.h"
On 2016/05/24 16:24:03, Srirama wrote:
> On 2016/05/23 22:53:56, jrummell wrote:
> > #include base/logging.h
> 
> Not able to include base/logging.h because of dependency rules.

Acknowledged. This is indeed strange, but looking at all the CL's for issue
596760 it looks like nobody changes the #includes.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp:318:
DVLOG(EME_LOG_LEVEL) << "setMediaKeys current(" << thisElement.m_mediaKeys.get()
<< "), new(" << mediaKeys << ")";
On 2016/05/24 16:24:03, Srirama wrote:
> On 2016/05/23 22:53:56, jrummell wrote:
> > std::hex and std::dec?
> 
> Why std::hex and std::dec? I guess, you want "this" pointer to be printed as
hex
> address? It is already printing in that format.

Acknowledged. I just looked for other uses of "<< this" and noticed that some
did the std::hex and std::dec. However, if it prints in hex this way already
then it is not needed.

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp
(right):

https://codereview.chromium.org/2001173002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/encryptedmedia/MediaKeySession.cpp:327:
DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << "MediaKeySession(" << this << ")";
On 2016/05/24 16:24:04, Srirama wrote:
> On 2016/05/23 22:53:56, jrummell wrote:
> > To keep the message consistent I think this needs to be "std::hex << this <<
> > std::dec". Maybe overload operator<< to return "MediaKeySession(xxxx)::" and
> > then use << this << __FUNCTION__?
> 
> DVLOG already prints filename and line number, so do we need
"MediaKeySession::"
> to be printed? We are just printing function name in HTMLMedia* file and other
> media files. So i think __FUNCTION__ is good enough. WDYT?

Acknowledged.

[Srirama, 2016-05-25 03:20:23]

Description was changed from

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522
==========

to

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522, 596760
==========

[Srirama, 2016-05-25 03:21:14]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2016-05-25 03:21:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2001173002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2001173002/60001

[commit-bot: I haz the power, 2016-05-25 03:25:35]

Description was changed from

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522, 596760
==========

to

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522, 596760
==========

[commit-bot: I haz the power, 2016-05-25 03:25:36]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-05-25 03:27:19]

Description was changed from

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522, 596760
==========

to

==========
encryptedmedia: Replace wtf/Assertions.h macros in favor of base/logging.h
macros

Replaced wtf/Assertions.h macros in favor of base/logging.h
macros in encryptedmedia module.

WTF_LOG -> DVLOG
ASSERT -> DCHECK
ASSERT_NOT_REACHED -> NOTREACHED

BUG=596522, 596760

Committed: https://crrev.com/060d0889ac68f2bd629ab3b684723dd32fce7a88
Cr-Commit-Position: refs/heads/master@{#395791}
==========

[commit-bot: I haz the power, 2016-05-25 03:27:20]

Patchset 2 (id:??) landed as
https://crrev.com/060d0889ac68f2bd629ab3b684723dd32fce7a88
Cr-Commit-Position: refs/heads/master@{#395791}