MD Settings: Second try at fixing ResetSettingsHandler crash

chrome/browser/ui/webui/settings/reset_settings_handler.cc

 // Copyright 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/ui/webui/settings/reset_settings_handler.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 127 matching lines @@
   bool send_settings = false;
   CHECK(args->GetBoolean(1, &send_settings));
   std::string request_origin_string;
   CHECK(args->GetString(2, &request_origin_string));
   reset_report::ChromeResetReport::ResetRequestOrigin request_origin =
       ResetRequestOriginFromString(request_origin_string);
 
   DCHECK(brandcode_.empty() || config_fetcher_);
   if (config_fetcher_ && config_fetcher_->IsActive()) {
     // Reset once the prefs are fetched.
-    config_fetcher_->SetCallback(base::Bind(
-        &ResetSettingsHandler::ResetProfile, weak_ptr_factory_.GetWeakPtr(),
-        callback_id, send_settings, request_origin));
+    config_fetcher_->SetCallback(base::Bind(&ResetSettingsHandler::ResetProfile,

[tommycli, 2017/03/30 15:26:26]

This just reverts the previous unsuccessful fix.

+                                            base::Unretained(this), callback_id,
+                                            send_settings, request_origin));
   } else {
     ResetProfile(callback_id, send_settings, request_origin);
   }
 }
 
 void ResetSettingsHandler::OnResetProfileSettingsDone(
     std::string callback_id,
     bool send_feedback,
     reset_report::ChromeResetReport::ResetRequestOrigin request_origin) {
   ResolveJavascriptCallback(base::Value(callback_id),
@@ skipping 60 matching lines @@
   chrome_prefs::ClearResetTime(profile_);
 }
 
 void ResetSettingsHandler::OnSettingsFetched() {
   DCHECK(config_fetcher_);
   DCHECK(!config_fetcher_->IsActive());
   // The master prefs is fetched. We are waiting for user pressing 'Reset'.
 }
 
 void ResetSettingsHandler::ResetProfile(
-    const std::string& callback_id,
+    const std::string callback_id,
     bool send_settings,
     reset_report::ChromeResetReport::ResetRequestOrigin request_origin) {
   DCHECK(!GetResetter()->IsActive());
 
   std::unique_ptr<BrandcodedDefaultSettings> default_settings;
   if (config_fetcher_) {
     DCHECK(!config_fetcher_->IsActive());
     default_settings = config_fetcher_->GetSettings();
     config_fetcher_.reset();
   } else {
     DCHECK(brandcode_.empty());
   }
 
   // If failed to fetch BrandcodedDefaultSettings or this is an organic
   // installation, use default settings.
   if (!default_settings)
     default_settings.reset(new BrandcodedDefaultSettings);
 
   GetResetter()->Reset(
       ProfileResetter::ALL, std::move(default_settings),
       base::Bind(&ResetSettingsHandler::OnResetProfileSettingsDone,
                  weak_ptr_factory_.GetWeakPtr(), callback_id, send_settings,

[dschuyler, 2017/03/30 22:32:07]

I'm missing how this is really different, since afaik a const reference is
copied by bind.
See
https://cs.chromium.org/chromium/src/docs/callback.md?type=cs&q=/docs/callbac...

My concern is that there is still something that needs fixing (if this is really
copying either way, then while this is unlikely to hurt, it wouldn't be a fix).

[dschuyler, 2017/03/30 22:56:01]

Just an update fyi note:
Tommy and I talked in person and it's the config_fetcher_.reset(); on line 240
that needs to be considered here - the owner of the callback data may be reset
while inside the callback.

                  request_origin));
   base::RecordAction(base::UserMetricsAction("ResetProfile"));
   UMA_HISTOGRAM_BOOLEAN("ProfileReset.SendFeedback", send_settings);
   UMA_HISTOGRAM_ENUMERATION(
       "ProfileReset.ResetRequestOrigin", request_origin,
       reset_report::ChromeResetReport::ResetRequestOrigin_MAX + 1);
 }
 
 ProfileResetter* ResetSettingsHandler::GetResetter() {
   if (!resetter_)
@@ skipping 38 matching lines @@
 void ResetSettingsHandler::OnShowPowerwashDialog(
      const base::ListValue* args) {
   UMA_HISTOGRAM_ENUMERATION(
       "Reset.ChromeOS.PowerwashDialogShown",
       chromeos::reset::DIALOG_FROM_OPTIONS,
       chromeos::reset::DIALOG_VIEW_TYPE_SIZE);
 }
 #endif  // defined(OS_CHROMEOS)
 
 }  // namespace settings