Gather more debug info for the switch away from page id for titles and state.

Gather more debug info for the switch away from page id for titles and state.

This is temporary and can be reverted after we observe crashes (or none).

BUG=369661
TEST=Watch for crashes in UpdateState and UpdateTitle

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

[Charlie Reis, 2015-11-04 23:59:45]

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

[Charlie Reis, 2015-11-04 23:59:46]

Avi: Can you review in general?  This revives
https://codereview.chromium.org/1263393002/ to see if the earlier causes of the
crashes have been resolved.  One more step toward killing page ID.  (I'll plan
to land this Thursday, so that it's not in the same Canary as
https://codereview.chromium.org/1425303002/.)

Robert: Can you review crash_keys.cc?

[Avi (use Gerrit), 2015-11-05 00:50:35]

LGTM with nitpickiness.

(Is this the same patch that I landed earlier? I guess I don't like my own
code.)

https://codereview.chromium.org/1410543014/diff/20001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1410543014/diff/20001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:3899: CHECK(new_entry);
I get that if the page id lookup failed, we wouldn't get this far, so we want
this to exist, but that seems a bit clever.

Perhaps on line 3892 we we remove the null check, gate the crash key value
settings on null, and put the null check after the CHECK_EQ.

(We already do this for the title update, below.)

[Robert Sesek, 2015-11-05 15:57:27]

crash_keys LGTM

[Charlie Reis, 2015-11-05 22:12:51]

https://codereview.chromium.org/1410543014/diff/20001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1410543014/diff/20001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:3899: CHECK(new_entry);
On 2015/11/05 00:50:35, Avi wrote:
> I get that if the page id lookup failed, we wouldn't get this far, so we want
> this to exist, but that seems a bit clever.
> 
> Perhaps on line 3892 we we remove the null check, gate the crash key value
> settings on null, and put the null check after the CHECK_EQ.
> 
> (We already do this for the title update, below.)

Done.

[Charlie Reis, 2015-11-05 22:12:56]

The CQ bit was checked by creis@chromium.org

[Charlie Reis, 2015-11-05 22:12:56]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1410543014/#ps40001
(title: "Avi's suggestions")

[commit-bot: I haz the power, 2015-11-05 22:13:48]

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

[commit-bot: I haz the power, 2015-11-05 22:47:43]

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

[commit-bot: I haz the power, 2015-11-05 22:47:44]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Charlie Reis, 2015-11-05 23:05:37]

https://codereview.chromium.org/1410543014/diff/20001/content/browser/web_con...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/1410543014/diff/20001/content/browser/web_con...
content/browser/web_contents/web_contents_impl.cc:3899: CHECK(new_entry);
On 2015/11/05 22:12:51, Charlie Reis (slow til 11-16) wrote:
> On 2015/11/05 00:50:35, Avi wrote:
> > I get that if the page id lookup failed, we wouldn't get this far, so we
want
> > this to exist, but that seems a bit clever.
> > 
> > Perhaps on line 3892 we we remove the null check, gate the crash key value
> > settings on null, and put the null check after the CHECK_EQ.
> > 
> > (We already do this for the title update, below.)
> 
> Done.

Oops, that should have been a null check at the end, rather than a CHECK.  Fixed
in patch set 4.

[Charlie Reis, 2015-11-05 23:05:44]

The CQ bit was checked by creis@chromium.org

[Charlie Reis, 2015-11-05 23:05:45]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
avi@chromium.org
Link to the patchset: https://codereview.chromium.org/1410543014/#ps60001
(title: "Fix null check")

[commit-bot: I haz the power, 2015-11-05 23:06:35]

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

[commit-bot: I haz the power, 2015-11-06 00:28:04]

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

[commit-bot: I haz the power, 2015-11-06 00:28:05]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Charlie Reis, 2015-11-06 00:28:26]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2015-11-06 00:29:43]

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

[commit-bot: I haz the power, 2015-11-06 00:58:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-06 01:00:03]

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

[Avi (use Gerrit), 2015-11-06 02:39:39]

On 2015/11/06 01:00:03, commit-bot: I haz the power wrote:
> Patchset 4 (id:??) landed as
> https://crrev.com/a67bd94373bfe81c29d2a0cfd7ae956f899eeece
> Cr-Commit-Position: refs/heads/master@{#358213}

Fingers crossed!

[Charlie Reis, 2015-11-06 19:01:31]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1412833009/ by creis@chromium.org.

The reason for reverting is: We're getting crashes for this on the 48.0.2556.0
canary (in both UpdateState and UpdateTitle)..