New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 616171 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Field trials activated in the GPU process do not propagate their state to the browser process

Project Member Reported by asvitk...@chromium.org, May 31 2016

Issue description

Field trials activated in the GPU process do not propagate their state to the browser process.

See discussion here:

https://bugs.chromium.org/p/chromium/issues/detail?id=609867#c11
 
So probably solution is to refactor the functionality to monitor field trials and notify browser process out of chrome_render_thread_observer.cc into a helper class and make both that observer and I guess maybe chrome_content_gpu_client.cc own a copy of that class?
I guess it's also received by chrome_render_message_filter.h on the browser side:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/renderer_host/chrome_render_message_filter.h&l=119

So we'd need an analogue of this for gpu process.

content/browser/gpu/gpu_process_host.h seems a good place to handle IPCs from the GPU process about field trial activations.
And there's also GpuChannelHost::MessageFilter.
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 2 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23974972332452d270ba2af7fad725ce1a5e397c

commit 23974972332452d270ba2af7fad725ce1a5e397c
Author: asvitkine <asvitkine@chromium.org>
Date: Thu Jun 02 06:34:39 2016

Refactor out a new class for syncing field trials in child processes.

This is split off out of chrome_render_thread_observer.cc so that
we can use it in other processes than the renderer. For example, in
the GPU process which now receives field trial and feature state
on the command-line. (That will be done in a follow-up CL.)

BUG= 616171 

Review-Url: https://codereview.chromium.org/2020413002
Cr-Commit-Position: refs/heads/master@{#397316}

[modify] https://crrev.com/23974972332452d270ba2af7fad725ce1a5e397c/chrome/chrome_common.gypi
[add] https://crrev.com/23974972332452d270ba2af7fad725ce1a5e397c/chrome/common/variations/child_process_field_trial_syncer.cc
[add] https://crrev.com/23974972332452d270ba2af7fad725ce1a5e397c/chrome/common/variations/child_process_field_trial_syncer.h
[modify] https://crrev.com/23974972332452d270ba2af7fad725ce1a5e397c/chrome/renderer/chrome_render_thread_observer.cc
[modify] https://crrev.com/23974972332452d270ba2af7fad725ce1a5e397c/chrome/renderer/chrome_render_thread_observer.h

Cc: shrike@chromium.org
Hello asvitkine@,

Happy to see this cl has landed. Any idea on when you'll be able to land the follow-up CL?

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5dc812a7851fa06da2ffd20f06c00aa553e1e19e

commit 5dc812a7851fa06da2ffd20f06c00aa553e1e19e
Author: asvitkine <asvitkine@chromium.org>
Date: Tue Jun 07 18:20:30 2016

Make field trials activated in the GPU process be reflected in browser.

This allows field trials that are queried in the GPU process be
reflected in UMA and crash reports. Currently, they are being
omitted.

This follows https://codereview.chromium.org/2020413002 - which
refactored this functionality out of the renderer process code, so that
it could be re-used (i.e. in this CL). One modification is made to that
refactoring, which is removing the sending of the IPC message from
the syncer class, since GPU and renderer processes require different
IPC messages.

BUG= 616171 

Review-Url: https://codereview.chromium.org/2034933002
Cr-Commit-Position: refs/heads/master@{#398345}

[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/common/variations/child_process_field_trial_syncer.cc
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/common/variations/child_process_field_trial_syncer.h
[add] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/common/variations/child_process_field_trial_syncer_unittest.cc
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/gpu/chrome_content_gpu_client.cc
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/gpu/chrome_content_gpu_client.h
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/renderer/chrome_render_thread_observer.cc
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/chrome/renderer/chrome_render_thread_observer.h
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/content/browser/gpu/gpu_process_host.h
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/content/common/gpu_host_messages.h
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/5dc812a7851fa06da2ffd20f06c00aa553e1e19e/content/public/gpu/content_gpu_client.h

I've now landed the change so it should work on TOT. The config change is still better - which Erik has submitted,  because we don't need to wait for dev push.
Great!
Status: Fixed (was: Assigned)

Sign in to add a comment