Do a nullptr cleanup in WebContentsImpl's unittest.

Do a nullptr cleanup in WebContentsImpl's unittest.

BUG=none
TEST=none

Committed: https://crrev.com/6511b9e83cc537f3c668c9bca978a6d44dffd811
Cr-Commit-Position: refs/heads/master@{#320775}

[Avi (use Gerrit), 2015-03-15 03:04:32]

avi@chromium.org changed reviewers:
+ creis@chromium.org

[Avi (use Gerrit), 2015-03-15 03:04:33]

I was hacking in this file and was doing this minor cleanup, and realized that I
needed to separate it from the big patch.

[Charlie Reis, 2015-03-16 18:41:25]

LGTM

https://codereview.chromium.org/1007123002/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl_unittest.cc (right):

https://codereview.chromium.org/1007123002/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_unittest.cc:436:
EXPECT_EQ(nullptr, contents()->GetPendingMainFrame());
Just curious to get more opinions on this topic-- what's your take on
EXPECT_EQ(nullptr, foo) vs EXPECT_FALSE(foo)?

I like that the former gives a more specific error message, though the latter is
more concise has been recommended to me in the past.  I'm fine either way here,
but it's good input for knowing what to recommend to others.

https://codereview.chromium.org/1007123002/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_unittest.cc:444: // Controller's
pending entry will have a nullptr site instance until we
I would find "have a null SiteInstance" easier to read, but I don't feel
strongly.

[Avi (use Gerrit), 2015-03-16 19:14:32]

https://codereview.chromium.org/1007123002/diff/1/content/browser/web_content...
File content/browser/web_contents/web_contents_impl_unittest.cc (right):

https://codereview.chromium.org/1007123002/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_unittest.cc:436:
EXPECT_EQ(nullptr, contents()->GetPendingMainFrame());
On 2015/03/16 18:41:24, Charlie Reis wrote:
> Just curious to get more opinions on this topic-- what's your take on
> EXPECT_EQ(nullptr, foo) vs EXPECT_FALSE(foo)?

My opinion is that EXPECT_FALSE is for boolean values.

> I like that the former gives a more specific error message, though the latter
is
> more concise has been recommended to me in the past.  I'm fine either way
here,
> but it's good input for knowing what to recommend to others.

NULL pointers have long been an issue with the macro-based EXPECT functions, but
now that we have nullptr, I don't see why we shouldn't use the EXPECTs as simply
as possible.

https://codereview.chromium.org/1007123002/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl_unittest.cc:444: // Controller's
pending entry will have a nullptr site instance until we
On 2015/03/16 18:41:24, Charlie Reis wrote:
> I would find "have a null SiteInstance" easier to read, but I don't feel
> strongly.

Done.

[Avi (use Gerrit), 2015-03-16 19:14:55]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2015-03-16 19:14:55]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org
Link to the patchset: https://codereview.chromium.org/1007123002/#ps20001
(title: "nits")

[commit-bot: I haz the power, 2015-03-16 19:15:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1007123002/20001

[commit-bot: I haz the power, 2015-03-16 19:35:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-16 19:36:55]

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