Return ERR_NOT_IMPLEMENTED when we reach SPDY stream creation code in a factory for WebSocket

Return ERR_NOT_IMPLEMENTED when we reach SPDY stream creation code in a factory for WebSocket

BUG=394434
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288118

[Adam Rice, 2014-08-06 03:42:24]

lgtm

[tyoshino (SeeGerritForStatus), 2014-08-06 04:05:08]

+jgraettinger

[Johnny, 2014-08-06 14:02:35]

Per the Chromium style guide [1], there shouldn't be a return here. How about a
CHECK instead, to force a crash? That will at least tell you if this path is
being hit.

[1]
http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-

[tyoshino (SeeGerritForStatus), 2014-08-07 03:32:51]

On 2014/08/06 14:02:35, Johnny wrote:
> Per the Chromium style guide [1], there shouldn't be a return here. How about
a
> CHECK instead, to force a crash? That will at least tell you if this path is
> being hit.
> 
> [1]
>
http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-

OK. I replaced it with CHECK() to identify the cause during the next dev. Beta
release is coming so at some point we might need to change this to just return
ERR_NOT_IMPLEMENTED not to crash.

BTW, HttpStreamFactoryImpl::Job::DoInitConnection() also contains code violating
the style guide.

[tyoshino (SeeGerritForStatus), 2014-08-07 08:31:28]

On 2014/08/07 03:32:51, tyoshino wrote:
> On 2014/08/06 14:02:35, Johnny wrote:
> > Per the Chromium style guide [1], there shouldn't be a return here. How
about
> a
> > CHECK instead, to force a crash? That will at least tell you if this path is
> > being hit.
> > 
> > [1]
> >
>
http://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREACHED-
> 
> OK. I replaced it with CHECK() to identify the cause during the next dev. Beta
> release is coming so at some point we might need to change this to just return
> ERR_NOT_IMPLEMENTED not to crash.
> 
> BTW, HttpStreamFactoryImpl::Job::DoInitConnection() also contains code
violating
> the style guide.

Hmm, we're close to M37 release, so I'd rather like to make this
ERR_NOT_IMPLEMENTED. Returning it prevents the callback which is hitting SEGV
from being invoked. We can also see whether the code is the cause or not by
looking at crash report. So, just ERR_NOT_IMPLEMENTED would be fine.

PTAL ps5

[Johnny, 2014-08-07 16:03:25]

lgtm

[tyoshino (SeeGerritForStatus), 2014-08-07 16:37:28]

The CQ bit was checked by tyoshino@chromium.org

[commit-bot: I haz the power, 2014-08-07 16:42:50]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/tyoshino@chromium.org/440293003/80001

[commit-bot: I haz the power, 2014-08-07 19:44:39]

Change committed as 288118