Fix registerProtocolHandler implementation on Windows 8 and 10.

chrome/browser/shell_integration_win.cc

 // Copyright (c) 2012 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/shell_integration_win.h"
 
 #include <windows.h>
 #include <shlwapi.h>
 #include <shobjidl.h>
 #include <propkey.h>  // Needs to come after shobjidl.h.
@@ skipping 25 matching lines @@
 #include "chrome/browser/policy/policy_path_parser.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/setup/setup_util.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
 #include "chrome/installer/util/install_util.h"
+#include "chrome/installer/util/scoped_user_protocol_entry.h"
 #include "chrome/installer/util/set_reg_value_work_item.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/installer/util/util_constants.h"
 #include "chrome/installer/util/work_item.h"
 #include "chrome/installer/util/work_item_list.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 
 using content::BrowserThread;
 
@@ skipping 174 matching lines @@
 // interaction early when it is clear that the user made a choice (e.g. http
 // and https for default browser).
 //
 // This helper class manages both the timer and the registry watchers and makes
 // sure the callback for the end of the settings interaction is only run once.
 // This class also manages its own lifetime.
 class OpenSystemSettingsHelper {
  public:
   // Begin the monitoring and will call |on_finished_callback| when done.
   // Takes in a null-terminated array of |protocols| whose registry keys must be
-  // watched.
+  // watched. The array must contain at least one element.
   static void Begin(const wchar_t* const protocols[],
                     const base::Closure& on_finished_callback) {
     new OpenSystemSettingsHelper(protocols, on_finished_callback);
   }
 
  private:
   // The reason the settings interaction concluded. Do not modify the ordering
   // because it is used for UMA.
   enum ConcludeReason { REGISTRY_WATCHER, TIMEOUT, NUM_CONCLUDE_REASON_TYPES };
 
   OpenSystemSettingsHelper(const wchar_t* const protocols[],
                            const base::Closure& on_finished_callback)
-      : on_finished_callback_(on_finished_callback), weak_ptr_factory_(this) {
+      : scoped_user_protocol_entry_(protocols[0]),
+        on_finished_callback_(on_finished_callback),
+        weak_ptr_factory_(this) {
     static const wchar_t kUrlAssociationFormat[] =
         L"SOFTWARE\\Microsoft\\Windows\\Shell\\Associations\\UrlAssociations\\"
         L"%ls\\UserChoice";
 
     // Remember the start time.
     start_time_ = base::TimeTicks::Now();
 
     for (const wchar_t* const* scan = &protocols[0]; *scan != nullptr; ++scan) {
       AddRegistryKeyWatcher(
           base::StringPrintf(kUrlAssociationFormat, *scan).c_str());
@@ skipping 44 matching lines @@
         new base::win::RegKey(HKEY_CURRENT_USER, key_path, KEY_NOTIFY));
 
     if (reg_key->Valid() &&
         reg_key->StartWatching(
             base::Bind(&OpenSystemSettingsHelper::OnRegistryKeyChanged,
                        weak_ptr_factory_.GetWeakPtr()))) {
       registry_key_watchers_.push_back(std::move(reg_key));
     }
   }
 
+  ScopedUserProtocolEntry scoped_user_protocol_entry_;

[grt (UTC plus 2), 2016/04/18 19:00:56]

nit: a comment here would be helpful to understand why this is here.

on second thought, does this need to be here in shell_integration? can it be
done inside Show*SystemUI for both the Win8 and Win10+ cases?

[grt (UTC plus 2), 2016/04/19 02:03:46]

as per face-to-face discussion, i understand this is needed here because the
cleanup must happen in the helper's dtor. i think this is okay.

are we now in a world where ShellUtil::ShowMakeChromeDefaultSystemUI and
ShowMakeChromeDefaultProtocolClientSystemUI should never be called directly on
Win10 since we always want the benefits of the helper? can we get rid of all
other callers of these two functions and paint them with colours too painful to
see so that noone ever calls them again (preferring the functions in
shell_integrtion)?

[Patrick Monette, 2016/04/19 14:36:13]

I added a comment. I think shell util is due for a refactoring. I'll do it in
another CL.

+
   // The function to call when the interaction with the system settings is
   // finished.
   base::Closure on_finished_callback_;
 
   // The number of time the registry key watchers must fire.
   int registry_watcher_count_ = 0;
 
   // There can be multiple registry key watchers as some settings modify
   // multiple protocol associations. e.g. Changing the default browser modifies
   // the http and https associations.
@@ skipping 354 matching lines @@
   if (!ShellUtil::ShowMakeChromeDefaultProtocolClientSystemUI(dist, chrome_exe,
                                                               wprotocol)) {
     LOG(ERROR) << "Failed to launch the set-default-client Windows UI.";
     return false;
   }
 
   VLOG(1) << "Set-default-client Windows UI completed.";
   return true;
 }
 
+void SetAsDefaultProtocolClientUsingSystemSettings(
+    const std::string& protocol,
+    const base::Closure& on_finished_callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
+
+  base::FilePath chrome_exe;
+  if (!PathService::Get(base::FILE_EXE, &chrome_exe)) {
+    NOTREACHED() << "Error getting app exe path";
+    on_finished_callback.Run();
+    return;
+  }
+
+  // The helper manages its own lifetime.
+  base::string16 wprotocol(base::UTF8ToUTF16(protocol));
+  static const wchar_t* const kProtocols[] = {wprotocol.c_str(), nullptr};
+  OpenSystemSettingsHelper::Begin(kProtocols, on_finished_callback);
+
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  ShellUtil::ShowMakeChromeDefaultProtocolClientSystemUI(dist, chrome_exe,
+                                                         wprotocol);
+}
+
 }  // namespace win
 
 }  // namespace shell_integration