Harden WebNavigation API against invalid navigation callbacks

Harden WebNavigation API against invalid navigation callbacks

Apparently, we get "DidCommitProvisionalLoad" events for frames where we
didn't see the start of the provisional load. While debugging this, make
the API more resillient against these kind of failures. Also,
consistently DCHECK() in such cases and add a user metrics action.

BUG=392718
R=marja@chromium.org,asvitkine@chromium.org

[jochen (gone - plz use gerrit), 2014-07-10 13:29:12]

+isherman

[marja, 2014-07-10 13:32:00]

lgtm

[Ilya Sherman, 2014-07-11 01:16:01]

https://codereview.chromium.org/380213003/diff/1/tools/metrics/actions/action...
File tools/metrics/actions/actions.xml (right):

https://codereview.chromium.org/380213003/diff/1/tools/metrics/actions/action...
tools/metrics/actions/actions.xml:2520: </action>
Do you need timestamps for this metric?  If not, I'd recommend instead logging
this as a histogram, alongside a built-in baseline.

[jochen (gone - plz use gerrit), 2014-07-11 09:04:14]

On 2014/07/11 at 01:16:01, isherman wrote:
>
https://codereview.chromium.org/380213003/diff/1/tools/metrics/actions/action...
> File tools/metrics/actions/actions.xml (right):
> 
>
https://codereview.chromium.org/380213003/diff/1/tools/metrics/actions/action...
> tools/metrics/actions/actions.xml:2520: </action>
> Do you need timestamps for this metric?  If not, I'd recommend instead logging
this as a histogram, alongside a built-in baseline.

can you clarify what you mean by built-in baseline?

Basically, this action is supposed to never happen (and we should probably
CHECK() here), and we use actions for other similar things (incorrect session
storage usage, incorrect referrers in the network stack)

[Ilya Sherman, 2014-07-11 23:09:56]

On 2014/07/11 09:04:14, jochen wrote:
> On 2014/07/11 at 01:16:01, isherman wrote:
> >
>
https://codereview.chromium.org/380213003/diff/1/tools/metrics/actions/action...
> > File tools/metrics/actions/actions.xml (right):
> > 
> >
>
https://codereview.chromium.org/380213003/diff/1/tools/metrics/actions/action...
> > tools/metrics/actions/actions.xml:2520: </action>
> > Do you need timestamps for this metric?  If not, I'd recommend instead
logging
> this as a histogram, alongside a built-in baseline.
> 
> can you clarify what you mean by built-in baseline?
> 
> Basically, this action is supposed to never happen (and we should probably
> CHECK() here), and we use actions for other similar things (incorrect session
> storage usage, incorrect referrers in the network stack)

If you think the code is truly unreachable, then why record a metric for it? 
Does anyone ever come back and look at these metrics?  If so, would it be easier
if they were all in the same place, e.g. all grouped under a histogram named
something like "Extensions.UnexpectedErrors"?

If you think that the code is reachable, but rarely, then the idea of a baseline
is to give context for what "rarely" means.  For example, if I think that the
Autofill server should never tell me that a field is a fax number, then a useful
baseline would be the number of forms or fields queried.  That way, if I see 100
times when I got back the surprising result, I can see if that's low (because I
query the Autofill server a bunch; maybe the response bytes are being corrupted
en route) or surprisingly high (because it turns out I don't query the Autofill
server as often as I thought).

Anyway, syntactically LGTM if you really prefer this approach... but I'd
strongly encourage you to consider whether you need this metric at all (i.e.
whether anyone will pay attention to it); and if so, whether there is a better
way to record it.

[jochen (gone - plz use gerrit), 2014-07-15 11:47:25]

closing this CL as dcheng@ wrote an alternative fix