Add unit test for WebRTCInternalsMessageHandler.

Add unit test for WebRTCInternalsMessageHandler.

BUG=548624
Committed: https://crrev.com/c369574d2aba37bfc3ffef8a5160e6644a1d5f72
Cr-Commit-Position: refs/heads/master@{#407479}

[Max Morin, 2016-07-21 19:22:18]

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

[commit-bot: I haz the power, 2016-07-21 19:22:44]

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-21 19:22:44]

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

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

Dry run: No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Max Morin, 2016-07-25 09:53:43]

Description was changed from

==========
Add unit test for WebRTCInternalsMessageHandler.

BUG=548624
==========

to

==========
Add unit test for WebRTCInternalsMessageHandler.

BUG=548624
==========

[Max Morin, 2016-07-25 09:53:44]

maxmorin@chromium.org changed reviewers:
+ tommi@chromium.org

[Max Morin, 2016-07-25 09:56:01]

PTAL.

[tommi (sloooow) - chröme, 2016-07-25 10:16:06]

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals.h (right):

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals.h:118: bool
should_block_power_saving);
remove 'explicit' (only used for a single argument ctor)

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals.h:161: const bool
should_block_power_saving_;
keep member variables together and at the bottom of the class

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc
(right):

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:38:
class WebRTCInternalsForTest : public NON_EXPORTED_BASE(WebRTCInternals) {
nit: empty line above this one

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:48:
const GURL kWebRTCURL =
either make this static or rename to webrtc_url_

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:50:
const GURL kExampleURL = GURL("http://www.example.com/");
same here.

Also, for initializing these variables, please just use the constructor instead
of the assignment operator

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:66:
std::unique_ptr<TestWebUI> web_UI;
web_ui_

[Max Morin, 2016-07-25 10:41:18]

PTAL.

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals.h (right):

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals.h:118: bool
should_block_power_saving);
On 2016/07/25 10:16:06, tommi-chrömium wrote:
> remove 'explicit' (only used for a single argument ctor)

Done.

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals.h:161: const bool
should_block_power_saving_;
On 2016/07/25 10:16:06, tommi-chrömium wrote:
> keep member variables together and at the bottom of the class

Done.

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc
(right):

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:38:
class WebRTCInternalsForTest : public NON_EXPORTED_BASE(WebRTCInternals) {
On 2016/07/25 10:16:06, tommi-chrömium wrote:
> nit: empty line above this one

Done.

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:48:
const GURL kWebRTCURL =
On 2016/07/25 10:16:06, tommi-chrömium wrote:
> either make this static or rename to webrtc_url_

Done.

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:50:
const GURL kExampleURL = GURL("http://www.example.com/");
On 2016/07/25 10:16:06, tommi-chrömium wrote:
> same here.
> 
> Also, for initializing these variables, please just use the constructor
instead
> of the assignment operator

Done.

https://codereview.chromium.org/2163983006/diff/20001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:66:
std::unique_ptr<TestWebUI> web_UI;
On 2016/07/25 10:16:06, tommi-chrömium wrote:
> web_ui_

Done.

[tommi (sloooow) - chröme, 2016-07-25 11:05:39]

lgtm with the below fixed

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals.cc (right):

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals.cc:502: if
(!should_block_power_saving_) {
remove the {} here. This file doesn't use them for single line conditions and
that's also more commonly the chromium style.

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc
(right):

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:51:
const GURL webrtc_url_{std::string("chrome://") +
I was actually expecting you to use () here since you weren't using {} for
initialization before.
See guidelines here for what to do:
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts?pli=1#TOC-...

I would have just done:

static const GURL kWebRTCURL(std::string("chrome://") +
kChromeUIWebRTCInternalsHost);
static const GURL kExampleURL("http://www.example.com/");

[Max Morin, 2016-07-25 12:02:57]

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals.cc (right):

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals.cc:502: if
(!should_block_power_saving_) {
On 2016/07/25 11:05:39, tommi-chrömium wrote:
> remove the {} here. This file doesn't use them for single line conditions and
> that's also more commonly the chromium style.

Done.

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
File content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc
(right):

https://codereview.chromium.org/2163983006/diff/40001/content/browser/media/w...
content/browser/media/webrtc/webrtc_internals_message_handler_unittest.cc:51:
const GURL webrtc_url_{std::string("chrome://") +
On 2016/07/25 11:05:39, tommi-chrömium wrote:
> I was actually expecting you to use () here since you weren't using {} for
> initialization before.
> See guidelines here for what to do:
>
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts?pli=1#TOC-...
> 
> I would have just done:
> 
> static const GURL kWebRTCURL(std::string("chrome://") +
> kChromeUIWebRTCInternalsHost);
> static const GURL kExampleURL("http://www.example.com/");

Using () makes these function declarations, hence the {}s. I'll just move them
to the test body.

[Max Morin, 2016-07-25 13:43:15]

The CQ bit was checked by maxmorin@chromium.org

[Max Morin, 2016-07-25 13:43:15]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/2163983006/#ps60001
(title: "Fix.")

[commit-bot: I haz the power, 2016-07-25 13:43:39]

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

[Max Morin, 2016-07-25 14:46:12]

The CQ bit was checked by maxmorin@chromium.org

[Max Morin, 2016-07-25 14:46:12]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@chromium.org
Link to the patchset: https://codereview.chromium.org/2163983006/#ps80001
(title: "Add NON_EXPORTED_BASE annotation.")

[commit-bot: I haz the power, 2016-07-25 14:46:17]

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-25 15:28:33]

Description was changed from

==========
Add unit test for WebRTCInternalsMessageHandler.

BUG=548624
==========

to

==========
Add unit test for WebRTCInternalsMessageHandler.

BUG=548624
==========

[commit-bot: I haz the power, 2016-07-25 15:28:34]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-07-25 15:30:13]

Description was changed from

==========
Add unit test for WebRTCInternalsMessageHandler.

BUG=548624
==========

to

==========
Add unit test for WebRTCInternalsMessageHandler.

BUG=548624

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

[commit-bot: I haz the power, 2016-07-25 15:30:14]

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