Fix potential crashes in NetworkHandler code

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/chrome_browser_main_chromeos.h"
 
 #include <stddef.h>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "ash/ash_switches.h"
 #include "ash/shell.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/linux_util.h"
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/sys_info.h"
 #include "base/task_runner_util.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_impl.h"
 #include "chrome/browser/browser_process_platform_part_chromeos.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/accessibility/accessibility_manager.h"
 #include "chrome/browser/chromeos/accessibility/magnification_manager.h"
 #include "chrome/browser/chromeos/accessibility/spoken_feedback_event_rewriter.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_launch_error.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_mode_idle_app_name_notification.h"
 #include "chrome/browser/chromeos/arc/arc_settings_bridge_impl.h"
 #include "chrome/browser/chromeos/boot_times_recorder.h"
@@ skipping 31 matching lines @@
 #include "chrome/browser/chromeos/power/power_prefs.h"
 #include "chrome/browser/chromeos/power/renderer_freezer.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/resource_reporter/resource_reporter.h"
 #include "chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h"
 #include "chrome/browser/chromeos/settings/device_settings_service.h"
 #include "chrome/browser/chromeos/status/data_promo_notification.h"
 #include "chrome/browser/chromeos/system/input_device_settings.h"
 #include "chrome/browser/chromeos/upgrade_detector_chromeos.h"
 #include "chrome/browser/defaults.h"
+#include "chrome/browser/io_thread.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/net/chrome_network_delegate.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/network_connect_delegate_chromeos.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
@@ skipping 727 matching lines @@
   // executed after execution of chrome::CloseAsh(), because some
   // parts of WebUI depends on NetworkPortalDetector.
   network_portal_detector::Shutdown();
 
   g_browser_process->platform_part()->DestroyChromeUserManager();
 
   g_browser_process->platform_part()->ShutdownSessionManager();
 }
 
 void ChromeBrowserMainPartsChromeos::PostDestroyThreads() {
+  // Destroy the ProxyConfigServiceImpl owned by IOThread before

[sky, 2016/01/05 23:17:11]

This just documents the code. Document why this order is important.

[stevenjb, 2016/01/06 00:43:04]

Clarified.

+  // shutting down NetworkHandler.
+  browser_process()->io_thread()->ShutdownPrefProxyConfigTracker();

[sky, 2016/01/05 23:17:11]

Won't this mean you're destroying the PrefProxyConfigTracker on a different
thread than it is currently destroyed? Is it setup to handle that?

[stevenjb, 2016/01/06 00:43:04]

No, it is created in IOThread() which is (somewhat unintuitively) created on the
UI thread. I added a few DCHECK_CURRENTLY_ON(UI) calls in io_thread.cc to
document that.

[sky, 2016/01/06 16:30:37]

I'm nervous and not familiar enough with this code to know if this is correct.
Can you get someone that knows about the threading of PrefProxyConfigTracker to
review this?

+
   // Destroy DBus services immediately after threads are stopped.
   dbus_services_.reset();
 
   ChromeBrowserMainPartsLinux::PostDestroyThreads();
 
   // Destroy DeviceSettingsService after g_browser_process.
   DeviceSettingsService::Shutdown();
 }
 
 }  //  namespace chromeos