Run the certificate management dialog with BaseShellDialogImpl.

chrome/browser/ui/webui/options/advanced_options_utils_win.cc

Index: chrome/browser/ui/webui/options/advanced_options_utils_win.cc
diff --git a/chrome/browser/ui/webui/options/advanced_options_utils_win.cc b/chrome/browser/ui/webui/options/advanced_options_utils_win.cc
index de9080e41af4f25e8b6f3c8bfcbf0786521984b4..8be378f57d9ba769ffee8cc46f730c55978d3fd3 100644
--- a/chrome/browser/ui/webui/options/advanced_options_utils_win.cc
+++ b/chrome/browser/ui/webui/options/advanced_options_utils_win.cc
@@ -10,11 +10,17 @@
 #include <shellapi.h>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
+#include "base/single_thread_task_runner.h"
 #include "base/threading/thread.h"
 #include "chrome/browser/browser_process.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
+#include "ui/shell_dialogs/base_shell_dialog_win.h"
 #include "ui/views/win/hwnd_util.h"
 
 using content::BrowserThread;
@@ -22,6 +28,56 @@ using content::WebContents;
 
 namespace options {
 
+namespace {
+
+// Shows a Windows certificate management dialog on the dialog thread.
+class ManageCertificatesDialog : public ui::BaseShellDialogImpl {
+ public:
+  explicit ManageCertificatesDialog(WebContents* web_contents)
+      : web_contents_(web_contents) {}

[mmenke, 2015/07/27 18:45:33]

optional:  Should we take the WebContents as a parameter to show instead?

[davidben, 2015/07/27 18:47:45]

I guess the question here is "what is a dialog object?". And, honestly, I have
no clue. BaseShellDialogImpl, despite being something you subclass, has no
actual state.

[mmenke, 2015/07/27 19:08:14]

I agree, and I'm with you on having no clue.

[davidben, 2015/07/27 19:24:19]

ananta? You wrote BaseShellDialogImpl. How are dialog classes in our code
supposed to work? (How much of what should be associated with the class and how
much should be associated with an individual call to Show() and passed along the
base::Binds?)

[ananta, 2015/07/27 19:59:23]

That is upto the caller. Please look at SelectFileDialogImpl for an example. For
this use case, I don't see why Show cannot take the web_contents_ in as an
argument. Alternatively you can pass in the parent HWND directly.

+
+  ~ManageCertificatesDialog() override {}
+
+  // Shows the dialog and calls |callback| when the dialog closes. The caller
+  // must ensure the ManageCertificatesDialog remains valid until then.
+  void Show(const base::Closure& callback) {
+    HWND parent =
+        views::HWNDForNativeWindow(web_contents_->GetTopLevelNativeWindow());
+    if (IsRunningDialogForOwner(parent)) {
+      base::MessageLoop::current()->PostTask(FROM_HERE, callback);
+      return;
+    }
+
+    RunState run_state = BeginRun(parent);
+    run_state.dialog_thread->task_runner()->PostTaskAndReply(
+        FROM_HERE, base::Bind(&ManageCertificatesDialog::ShowOnDialogThread,
+                              base::Unretained(this), run_state),
+        base::Bind(&ManageCertificatesDialog::OnDialogClosed,
+                   base::Unretained(this), run_state, callback));
+  }
+
+ private:
+  void ShowOnDialogThread(const RunState& run_state) {
+    CRYPTUI_CERT_MGR_STRUCT cert_mgr = {0};
+    cert_mgr.dwSize = sizeof(CRYPTUI_CERT_MGR_STRUCT);
+    cert_mgr.hwndParent = run_state.owner;
+    ::CryptUIDlgCertMgr(&cert_mgr);
+  }
+
+  void OnDialogClosed(const RunState& run_state,
+                      const base::Closure& callback) {

[mmenke, 2015/07/27 19:08:14]

Should we have a weak pointer here?  Or this should be static.

[mmenke, 2015/07/27 19:13:30]

Oh, right...with your ownership model, not needed.

+    EndRun(run_state);
+    // May delete |this|.
+    callback.Run();
+  }
+
+  WebContents* web_contents_;
+
+  DISALLOW_COPY_AND_ASSIGN(ManageCertificatesDialog);
+};
+
+}  // namespace
+
 // Callback that opens the Internet Options control panel dialog with the
 // Connections tab selected.
 void OpenConnectionDialogCallback() {
@@ -58,11 +114,9 @@ void AdvancedOptionsUtilities::ShowNetworkProxySettings(
 
 void AdvancedOptionsUtilities::ShowManageSSLCertificates(
       WebContents* web_contents) {
-  CRYPTUI_CERT_MGR_STRUCT cert_mgr = { 0 };
-  cert_mgr.dwSize = sizeof(CRYPTUI_CERT_MGR_STRUCT);
-  cert_mgr.hwndParent = views::HWNDForNativeWindow(
-      web_contents->GetTopLevelNativeWindow());
-  ::CryptUIDlgCertMgr(&cert_mgr);
+  ManageCertificatesDialog* dialog = new ManageCertificatesDialog(web_contents);
+  dialog->Show(
+      base::Bind(&base::DeletePointer<ManageCertificatesDialog>, dialog));
 }
 
 }  // namespace options