Issue 17741010: Make ChildThread::current() and ChildProcess::current() only work on the main thread of the child...

Issue 17741010: Make ChildThread::current() and ChildProcess::current() only work on the main thread of the child... (Closed)

Created:
7 years, 6 months ago by jam

Modified:
7 years, 5 months ago

Reviewers:
Feng Qian, scottmg

CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam

Base URL:
svn://chrome-svn/chrome/trunk/src/

Visibility:
Public.

More Reviews

Description

Make ChildThread::current() and ChildProcess::current() only work on the main thread of the child process. In single-process mode, there would be multiple ChildThread objects each using thread local storage to get their value. Remove ChildThread::IsWebFrameValid since it's not used anymore. BUG=246884 R=scottmg@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=208777

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : also ChildProcess::current() #

Total comments: 5

Patch Set 4 : fix crash #

Patch Set 5 : fix crash #

Patch Set 6 : sync and also fix gpu single process case #

Total comments: 2

Created: 7 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+105 lines, -107 lines)			Patch
M	content/app/android/child_process_service.cc	View	1 2 3 4 5	4 chunks	+15 lines, -26 lines	0 comments	Download
M	content/browser/gpu/gpu_process_host.cc	View	1 2 3 4 5	2 chunks	+5 lines, -13 lines	0 comments	Download
M	content/child/child_process.h	View	1 2 3 4 5	1 chunk	+2 lines, -1 line	0 comments	Download
M	content/child/child_process.cc	View	1 2 3 4 5	6 chunks	+14 lines, -10 lines	0 comments	Download
M	content/child/child_thread.h	View	1 2 3 4 5	2 chunks	+6 lines, -5 lines	0 comments	Download
M	content/child/child_thread.cc	View	1 2 3 4 5	7 chunks	+28 lines, -5 lines	2 comments	Download
M	content/renderer/pepper/pepper_hung_plugin_filter.cc	View	1 2 3 4 5	2 chunks	+2 lines, -1 line	0 comments	Download
M	content/renderer/pepper/pepper_platform_audio_output_impl.cc	View	1 2 3 4 5	1 chunk	+0 lines, -9 lines	0 comments	Download
M	content/renderer/render_thread_impl.h	View	1 2 3 4 5	4 chunks	+22 lines, -20 lines	0 comments	Download
M	content/renderer/render_thread_impl.cc	View	1 2 3 4 5	6 chunks	+11 lines, -17 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

scottmg

lgtm if there's nothing to be done about the comments. https://codereview.chromium.org/17741010/diff/83001/content/child/child_process.cc File content/child/child_process.cc (right): https://codereview.chromium.org/17741010/diff/83001/content/child/child_process.cc#newcode100 ...

7 years, 6 months ago (2013-06-26 15:21:50 UTC) #2

jam

https://codereview.chromium.org/17741010/diff/83001/content/child/child_process.cc File content/child/child_process.cc (right): https://codereview.chromium.org/17741010/diff/83001/content/child/child_process.cc#newcode100 content/child/child_process.cc:100: return g_lazy_tls.Pointer()->Get(); On 2013/06/26 15:21:50, scottmg wrote: > is ...

7 years, 6 months ago (2013-06-26 17:23:08 UTC) #3

Feng Qian

Thanks John, see my inline comment on Android platform. https://codereview.chromium.org/17741010/diff/137001/content/child/child_thread.cc File content/child/child_thread.cc (right): https://codereview.chromium.org/17741010/diff/137001/content/child/child_thread.cc#newcode89 content/child/child_thread.cc:89: ...

7 years, 5 months ago (2013-06-28 17:12:14 UTC) #4

jam

7 years, 5 months ago (2013-07-01 19:29:26 UTC) #5

Message was sent while issue was closed.

https://codereview.chromium.org/17741010/diff/137001/content/child/child_thre...
File content/child/child_thread.cc (right):

https://codereview.chromium.org/17741010/diff/137001/content/child/child_thre...
content/child/child_thread.cc:89: ChildThread* g_child_thread;
On 2013/06/28 17:12:14, Feng Qian wrote:
> @jam, won't this be a race condition when both Render and GpuThread are child
> threads on Android?

this is only called by content/app/android/child_process_service.cc, it's my
understanding that this is code which only runs in the multi-process path. am i
mistaken?

Expand Messages | Collapse Messages