Fix hanging on browser shutdown.

Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and makes calls to |gconf_client_*| API at the main thread.

BUG=486077
Committed: https://crrev.com/c7b954b3dd20dfd349de5d33468f405451452051
Cr-Commit-Position: refs/heads/master@{#394372}

[Andrey Kraynov, 2016-05-17 17:58:46]

Description was changed from

==========
Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and place a call to |gconf_client_*| API at the main thread.

BUG=486077
==========

to

==========
Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and makes calls to |gconf_client_*| API at the main thread.

BUG=486077
==========

[Andrey Kraynov, 2016-05-17 18:27:58]

iceman@yandex-team.ru changed reviewers:
+ dmazzoni@chromium.org

[Andrey Kraynov, 2016-05-17 18:27:59]

Hi!

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

But at the moment, it can be called from non-UI thread(from a worker thread of
the SequencedWorkerPool).
When the browser process is shutting down, it should wait for all blocking
threads, but one thread with the call to |gconf_client_get_bool| can randomly
hang.

That can lead to a hang with the following stack trace:

Thread 1 (Thread 0x7fbf1c473a00 (LWP 31382)):
#0  pthread_cond_timedwait@@GLIBC_2.3.2 () at
../nptl/sysdeps/unix/sysv/linux/x86_64/pthread_cond_timedwait.S:238
#1  0x00007fbf3936095f in base::ConditionVariable::TimedWait
(this=0x1833c59278d8, max_time=...) at
../../base/synchronization/condition_variable_posix.cc:117
#2  0x00007fbf393ad232 in base::SequencedWorkerPool::Inner::Shutdown
(this=0x1833c5927860, max_new_blocking_tasks_after_shutdown=1000) at
../../base/threading/sequenced_worker_pool.cc:777
#3  0x00007fbf393af481 in base::SequencedWorkerPool::Shutdown
(this=0x1833c5bbfd40, max_new_blocking_tasks_after_shutdown=1000) at
../../base/threading/sequenced_worker_pool.cc:1429
#4  0x00007fbf33b23eee in content::BrowserThreadImpl::ShutdownThreadPool () at
../../content/browser/browser_thread_impl.cc:143
#5  0x00007fbf33b06bc7 in content::BrowserMainLoop::ShutdownThreadsAndCleanUp
(this=0x1833c5792560) at ../../content/browser/browser_main_loop.cc:1143
#6  0x00007fbf33b103a3 in content::BrowserMainRunnerImpl::Shutdown
(this=0x1833c592b6e0) at ../../content/browser/browser_main_runner.cc:210
#7  0x00007fbf33aff333 in content::BrowserMain (parameters=...) at
../../content/browser/browser_main.cc:48
#8  0x00007fbf338ebe96 in content::RunNamedProcessTypeMain (process_type=...,
main_function_params=..., delegate=0x1833c561aaa0) at
../../content/app/content_main_runner.cc:381
#9  0x00007fbf338ee229 in content::ContentMainRunnerImpl::Run
(this=0x1833c5785da0) at ../../content/app/content_main_runner.cc:742
#10 0x00007fbf338eb212 in content::ContentMain (params=...) at
../../content/app/content_main.cc:20

...

Thread 4 (Thread 0x7fbf0d48e700 (LWP 31412)):
#0  0x00007fbf266ab3bd in read () at ../sysdeps/unix/syscall-template.S:81
#1  0x00007fbf2358a6c7 in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#2  0x00007fbf2358d681 in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#3  0x00007fbf2358d9c9 in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#4  0x00007fbf23582d3d in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#5  0x00007fbf23582c1f in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#6  0x00007fbf2356f05b in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#7  0x00007fbf2356ad58 in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
#8  0x00007fbf219ddac5 in ?? () from /usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#9  0x00007fbf219ddcc9 in ?? () from /usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#10 0x00007fbf219ddd82 in ?? () from /usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#11 0x00007fbf219df1a8 in gconf_engine_get_fuller () from
/usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#12 0x00007fbf219df58d in gconf_engine_get_entry () from
/usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#13 0x00007fbf219d9d2e in ?? () from /usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#14 0x00007fbf219dc13d in ?? () from /usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#15 0x00007fbf219dc7fb in gconf_client_get_bool () from
/usr/lib/x86_64-linux-gnu/libgconf-2.so.4
#16 0x00007fbf3999412d in
ui::AtkUtilAuraLinux::CheckPlatformAccessibilitySupportOnFileThread
(this=0x1833c681eb60) at
../../ui/accessibility/platform/atk_util_auralinux.cc:210
#17 0x00007fbf39993d0d in
ui::AtkUtilAuraLinux::CheckIfAccessibilityIsEnabledOnFileThread
(this=0x1833c681eb60) at
../../ui/accessibility/platform/atk_util_auralinux.cc:190
...

I was able to reproduce this hang on my Linux machine by running browser_tests
repeatedly:
$ ./testing/xvfb.py ./out/Debug ./out/Debug/browser_tests
--gtest_filter="CacheCounterTest.PrefChanged" --gtest_repeat=100

So I decided to partially revert CL https://codereview.chromium.org/1640273002
and make calls to |gconf_client_*| API at the main thread.
As far as I can see, it helps.

It seems that this hang can affect a lot of browser tests and make them flaky.
Also it can affect the Chromium browser itself, see
https://bugs.chromium.org/p/chromium/issues/detail?id=486077

I'm not an experienced Linux developer, and it will be great if someone could
take a look at this fix.

[dmazzoni, 2016-05-17 19:01:16]

lgtm

I think when we tried this before we hit a performance regression,
but I don't see a good alternative.

[Andrey Kraynov, 2016-05-18 09:00:15]

The CQ bit was checked by iceman@yandex-team.ru

[commit-bot: I haz the power, 2016-05-18 09:00:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1990453002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1990453002/1

[Andrey Kraynov, 2016-05-18 09:10:17]

On 2016/05/17 19:01:16, dmazzoni wrote:
> lgtm

Great, thanks!
 
> I think when we tried this before we hit a performance regression,
> but I don't see a good alternative.

I thought that it is possible to move all GConf stuff to a dedicated thread with
it's own GMainContext and query that thread for settings values.
But I'm not sure about that.

Also, what do you think about https://bugzilla.gnome.org/show_bug.cgi?id=683830
?
It says that dbus_g_thread_init() should be called before establishing the
connection to DBus.
Should we do that?

[commit-bot: I haz the power, 2016-05-18 09:53:40]

Description was changed from

==========
Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and makes calls to |gconf_client_*| API at the main thread.

BUG=486077
==========

to

==========
Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and makes calls to |gconf_client_*| API at the main thread.

BUG=486077
==========

[commit-bot: I haz the power, 2016-05-18 09:53:46]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-18 09:54:56]

Description was changed from

==========
Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and makes calls to |gconf_client_*| API at the main thread.

BUG=486077
==========

to

==========
Fix hanging on browser shutdown.

It seems that |gconf_client_*| API should be called exactly from the main thread
(which is UI thread), for example see
https://bugzilla.mozilla.org/show_bug.cgi?id=518244

This CL partially reverts CL https://codereview.chromium.org/1640273002
and makes calls to |gconf_client_*| API at the main thread.

BUG=486077

Committed: https://crrev.com/c7b954b3dd20dfd349de5d33468f405451452051
Cr-Commit-Position: refs/heads/master@{#394372}
==========

[commit-bot: I haz the power, 2016-05-18 09:54:57]

Patchset 1 (id:??) landed as
https://crrev.com/c7b954b3dd20dfd349de5d33468f405451452051
Cr-Commit-Position: refs/heads/master@{#394372}

[Finnur, 2016-05-18 13:00:30]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1993763002/ by finnur@chromium.org.

The reason for reverting is: Judging by the CL description I suspect this is
causing a shutdown crash on the Linux ChromiumOS Tests (1) bot:

https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%...

Reverting to see if that's correct..