Use DCHECK instead of LOG(ERROR) in embedded_worker_registry.cc

Use DCHECK instead of LOG(ERROR) in  embedded_worker_registry.cc

LOG statements pollute output.

BUG=N/A

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

[horo, 2014-08-21 01:48:37]

nhiroki@
Could you please review?

[nhiroki, 2014-08-21 02:13:18]

LGTM with nits.

CL title: "\" -> "/"

https://codereview.chromium.org/489303002/diff/1/content/browser/service_work...
File content/browser/service_worker/embedded_worker_registry.cc (right):

https://codereview.chromium.org/489303002/diff/1/content/browser/service_work...
content/browser/service_worker/embedded_worker_registry.cc:75:
DCHECK(found->second->process_id() == process_id);
Can you use DCHECK_EQ instead here and elsewhere?

[horo, 2014-08-21 03:56:39]

Done.

https://codereview.chromium.org/489303002/diff/1/content/browser/service_work...
File content/browser/service_worker/embedded_worker_registry.cc (right):

https://codereview.chromium.org/489303002/diff/1/content/browser/service_work...
content/browser/service_worker/embedded_worker_registry.cc:75:
DCHECK(found->second->process_id() == process_id);
On 2014/08/21 02:13:18, nhiroki wrote:
> Can you use DCHECK_EQ instead here and elsewhere?

Done.

[horo, 2014-08-21 03:56:48]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-08-21 03:57:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/horo@chromium.org/489303002/20001

[commit-bot: I haz the power, 2014-08-21 10:43:29]

Committed patchset #2 (20001) as 291038

[falken, 2014-11-06 02:07:53]

falken@chromium.org changed reviewers:
+ falken@chromium.org

[falken, 2014-11-06 02:07:54]

https://codereview.chromium.org/489303002/diff/20001/content/browser/service_...
File content/browser/service_worker/embedded_worker_registry.cc (right):

https://codereview.chromium.org/489303002/diff/20001/content/browser/service_...
content/browser/service_worker/embedded_worker_registry.cc:57: if (found ==
worker_map_.end())
Typically we should not have constructions of the form[1]:

DCHECK(foo);
if (!foo) { ... }

[1]
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/c2X0D1Z6X2o/ddkFL...

I would like to remove this handling of an impossible condition. However, this
change seemed motivated by log output spam, which would mean these are not
impossible conditions. That's a bug we better fix.

Do you remember what motivated this change? Do we see these DCHECKs occurring
still?

[falken, 2014-11-06 02:18:10]

On 2014/11/06 02:07:54, falken wrote:
>
https://codereview.chromium.org/489303002/diff/20001/content/browser/service_...
> File content/browser/service_worker/embedded_worker_registry.cc (right):
> 
>
https://codereview.chromium.org/489303002/diff/20001/content/browser/service_...
> content/browser/service_worker/embedded_worker_registry.cc:57: if (found ==
> worker_map_.end())
> Typically we should not have constructions of the form[1]:
> 
> DCHECK(foo);
> if (!foo) { ... }
> 
> [1]
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/c2X0D1Z6X2o/ddkFL...
> 
> I would like to remove this handling of an impossible condition. However, this
> change seemed motivated by log output spam, which would mean these are not
> impossible conditions. That's a bug we better fix.
> 
> Do you remember what motivated this change? Do we see these DCHECKs occurring
> still?

Actually since this input comes from the renderer, I think we should terminate
the renderer if it violates our assumptions rather than a DCHECK.

[horo, 2014-11-06 06:23:15]

On 2014/11/06 02:18:10, falken wrote:
> On 2014/11/06 02:07:54, falken wrote:
> >
>
https://codereview.chromium.org/489303002/diff/20001/content/browser/service_...
> > File content/browser/service_worker/embedded_worker_registry.cc (right):
> > 
> >
>
https://codereview.chromium.org/489303002/diff/20001/content/browser/service_...
> > content/browser/service_worker/embedded_worker_registry.cc:57: if (found ==
> > worker_map_.end())
> > Typically we should not have constructions of the form[1]:
> > 
> > DCHECK(foo);
> > if (!foo) { ... }
> > 
> > [1]
> >
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/c2X0D1Z6X2o/ddkFL...
> > 
> > I would like to remove this handling of an impossible condition. However,
this
> > change seemed motivated by log output spam, which would mean these are not
> > impossible conditions. That's a bug we better fix.
> > 
> > Do you remember what motivated this change? Do we see these DCHECKs
occurring
> > still?
> 
> Actually since this input comes from the renderer, I think we should terminate
> the renderer if it violates our assumptions rather than a DCHECK.

Yes. This message is from the renderer. So we should terminate the invalid
renderer.