Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(166)

Issue 10979048: Fix potential threading issues in NetworkChangeNotifierAndroid. (Closed)

Created:
8 years, 2 months ago by Philippe
Modified:
8 years, 2 months ago
Reviewers:
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, cbentzel+watch_chromium.org, jam
Visibility:
Public.

Description

Fix potential threading issues in NetworkChangeNotifierAndroid. NetworkChangeNotifierAndroid's constructor can be called on any thread. It calls some Java code (still on any thread) initializing a singleton. This singleton is also accessed from the UI thread on the application side. While in practice it didn't seem to be a problem it seems safer to perform all the JNI calls on a single thread (UI thread) to avoid accessing mutable state from multiple threads. Another potential issue with the current implementation was that NetworkChangeNotifierAndroid could be deleted on the network thread while it was receiving at the same time a notification on the UI thread. That could have led to a user after free. This CL addresses these two issues by introducing a Delegate who outlives NetworkChangeNotifierAndroid and defers JNI calls to the UI thread.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+183 lines, -55 lines) Patch
M chrome/browser/chrome_browser_main_android.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M content/shell/shell_browser_main_parts.cc View 2 chunks +5 lines, -4 lines 0 comments Download
M net/android/java/src/org/chromium/net/NetworkChangeNotifier.java View 1 chunk +2 lines, -2 lines 0 comments Download
M net/android/network_change_notifier_android.h View 1 chunk +27 lines, -14 lines 0 comments Download
M net/android/network_change_notifier_android.cc View 4 chunks +127 lines, -28 lines 0 comments Download
M net/android/network_change_notifier_android_unittest.cc View 1 chunk +3 lines, -1 line 0 comments Download
M net/android/network_change_notifier_factory_android.h View 2 chunks +10 lines, -1 line 0 comments Download
M net/android/network_change_notifier_factory_android.cc View 1 chunk +5 lines, -2 lines 0 comments Download

Powered by Google App Engine
This is Rietveld 408576698