Fix crash with IPC_MESSAGE_LOG_ENABLED when running in single-process mode or in-process-gpu. The …

Fix crash with IPC_MESSAGE_LOG_ENABLED when running in single-process mode or in-process-gpu.  The ipc_logging class is not thread safe so when when running in these modes the queued_logs_ were getting smashed.  Besides, there is no reason to queue these logs when running all in the same process.

BUG=none

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

[atreat, 2013-10-23 18:41:02]

The queued logs for ipc_logging don't make sense for in process threads.  This
patch fixes crash when running in process mode(s) with ipc logging enabled. 
Please review.

[Avi (use Gerrit), 2013-10-23 19:15:15]

I don't know enough about threading to comment. Will stamp if needed.

[atreat, 2013-10-23 20:34:41]

On 2013/10/23 19:15:15, Avi wrote:
> I don't know enough about threading to comment. Will stamp if needed.

Can you recommend a good reviewer?

[Avi (use Gerrit), 2013-10-23 20:49:43]

Ask John.

[jam, 2013-10-24 16:49:03]

https://codereview.chromium.org/32283003/diff/1/content/child/child_thread.cc
File content/child/child_thread.cc (right):

https://codereview.chromium.org/32283003/diff/1/content/child/child_thread.cc...
content/child/child_thread.cc:290: DCHECK(in_browser_process_ ||
base::MessageLoop::current() == message_loop());
why is this needed?

[atreat, 2013-10-24 16:56:36]

On 2013/10/24 16:49:03, jam wrote:
> https://codereview.chromium.org/32283003/diff/1/content/child/child_thread.cc
> File content/child/child_thread.cc (right):
> 
>
https://codereview.chromium.org/32283003/diff/1/content/child/child_thread.cc...
> content/child/child_thread.cc:290: DCHECK(in_browser_process_ ||
> base::MessageLoop::current() == message_loop());
> why is this needed?

That DCHECK gets hit when running in single process mode.  I think because the
child_thread thinks it is always the main thread of the process, but this isn't
true in single process mode.  At least, that is what I understand the DCHECK to
imply which isn't true.  I did look at git blame to see who/why this DCHECK was
added, but couldn't find an answer as it said 'initial commit'.

Anyway, the DCHECK is either invalid or getting triggered right now in single
process mode with logging enabled.

[jam, 2013-10-24 20:34:51]

On 2013/10/24 16:56:36, atreat wrote:
> On 2013/10/24 16:49:03, jam wrote:
> >
https://codereview.chromium.org/32283003/diff/1/content/child/child_thread.cc
> > File content/child/child_thread.cc (right):
> > 
> >
>
https://codereview.chromium.org/32283003/diff/1/content/child/child_thread.cc...
> > content/child/child_thread.cc:290: DCHECK(in_browser_process_ ||
> > base::MessageLoop::current() == message_loop());
> > why is this needed?
> 
> That DCHECK gets hit when running in single process mode.  I think because the
> child_thread thinks it is always the main thread of the process, but this
isn't
> true in single process mode.  At least, that is what I understand the DCHECK
to
> imply which isn't true.  I did look at git blame to see who/why this DCHECK
was
> added, but couldn't find an answer as it said 'initial commit'.
> 
> Anyway, the DCHECK is either invalid or getting triggered right now in single
> process mode with logging enabled.

can you attach a callstack? channel_ is not thread safe, so if it's being hit
this could be a problem. i don't see this being hit in single process mode..

[atreat, 2013-10-28 15:38:38]

On 2013/10/24 20:34:51, jam wrote:
> can you attach a callstack? channel_ is not thread safe, so if it's being hit
> this could be a problem. i don't see this being hit in single process mode..

Turns out that the DCHECK is triggered *without* the rest of this change.  In
other words, this patch fixes the issue by eliminating the IPC message for IPC
logs when the child_thread is being run in the same process as the browser
thread.

[jam, 2013-10-28 16:47:54]

lgtm

[commit-bot: I haz the power, 2013-10-28 17:01:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/32283003/80001

[commit-bot: I haz the power, 2013-10-28 19:29:34]

Change committed as 231374