Change the system-level EULA dialog to not use GET parameters with res:// urls. Instead use the d...

chrome/installer/util/html_dialog_impl.cc

Index: chrome/installer/util/html_dialog_impl.cc
===================================================================
--- chrome/installer/util/html_dialog_impl.cc	(revision 91469)
+++ chrome/installer/util/html_dialog_impl.cc	(working copy)
@@ -1,23 +1,17 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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 <windows.h>
+

[grt (UTC plus 2), 2011/07/04 18:42:04]

remove blank line?

[robertshield, 2011/07/04 19:12:31]

Done.

+#include <mshtmhst.h>
 #include <urlmon.h>
 
+#include "base/win/scoped_variant.h"
 #include "chrome/installer/util/html_dialog.h"
 
 #pragma comment(lib, "urlmon.lib")
 
-namespace {
-// Signature of MSHTML.DLL ShowHTMLDlg.
-typedef HRESULT (CALLBACK *ShowHTMLDlg)(HWND parent_hwnd,
-                                        IMoniker *moniker,
-                                        VARIANT *in_args,
-                                        TCHAR *options,
-                                        VARIANT *out_args);
-}  // namespace.
-
 namespace installer {
 
 // Windows implementation of the HTML dialog class. The main danger with
@@ -47,7 +41,9 @@
 
 class HTMLDialogWin : public HTMLDialog {
  public:
-  explicit HTMLDialogWin(const std::wstring& url) : url_(url) {
+  explicit HTMLDialogWin(const std::wstring& url,

[grt (UTC plus 2), 2011/07/04 18:42:04]

remove "explicit"

[robertshield, 2011/07/04 19:12:31]

Done.

+                         const std::wstring& param) : url_(url),
+                                                      param_(param) {
     if (!mshtml_)
        mshtml_ = LoadLibrary(L"MSHTML.DLL");
   }
@@ -70,13 +66,15 @@
   static LRESULT CALLBACK MsgFilter(int code, WPARAM wParam, LPARAM lParam);
 
   std::wstring url_;
+  std::wstring param_;
   static HHOOK hook_;
   static HINSTANCE mshtml_;
   static CustomizationCallback* callback_;
 };
 
-HTMLDialog* CreateNativeHTMLDialog(const std::wstring& url) {
-  return new HTMLDialogWin(url);
+HTMLDialog* CreateNativeHTMLDialog(const std::wstring& url,
+                                   const std::wstring& param) {
+  return new HTMLDialogWin(url, param);
 }
 
 HHOOK HTMLDialogWin::hook_ = NULL;
@@ -106,13 +104,14 @@
                                      int* result) {
   if (!mshtml_)
     return false;
-  ShowHTMLDlg show_html_dialog =
-      reinterpret_cast<ShowHTMLDlg>(GetProcAddress(mshtml_, "ShowHTMLDialog"));
+  SHOWHTMLDIALOGFN* show_html_dialog =
+      reinterpret_cast<SHOWHTMLDIALOGFN*>(
+          GetProcAddress(mshtml_, "ShowHTMLDialog"));
   if (!show_html_dialog)
     return false;
 
   IMoniker *url_moniker = NULL;
-  ::CreateURLMoniker(NULL, url_.c_str(), &url_moniker);
+  ::CreateURLMonikerEx(NULL, url_.c_str(), &url_moniker, URL_MK_UNIFORM);
   if (!url_moniker)
     return false;
 
@@ -126,11 +125,19 @@
       callback_ = callback;
   }
 
+  // Pass our parameter to the dialog in the dialogArguments property of
+  // the window object.
+  base::win::ScopedVariant dialog_args(param_.c_str());

[grt (UTC plus 2), 2011/07/04 18:42:04]

nit: dialog_args(param_.data(), param_.size()) will avoid an extra call to
strlen.

[robertshield, 2011/07/04 19:12:31]

It would avoid an extra call, but it feels like premature optimization and (you
can correct me if I'm wrong) I'm not sure it is guaranteed to work for empty
strings: basic_string::data() is documented as returning for an empty string "a
non-null pointer that cannot be dereferenced" and I'm unsure if there are any
guarantees as to how SysAllocStringLen() called with length 0 would deal with
that. 

I'd prefer to leave this as is if you don't strongly object since I feel the
above is clearer, with less stuff to worry about and if MSHTML dialogs are ever
created in bulk, then the extra strlen() call will not be the source of our
performance concerns.

[grt (UTC plus 2), 2011/07/04 19:24:19]

SysAllocStringLen is broken if it reads memory beyond what it's told.  You could
use c_str() rather than data() if that's expected.  A better move would be to
add a const std::wstring& overload to ScopedVariant that did the right thing,
but I'm 99.9999% positive you don't want to do that in this CL.  :-)
Works for me.

+
   VARIANT v_result;
   ::VariantInit(&v_result);
 
   // Creates the window with the embedded IE control in a modal loop.
-  HRESULT hr = show_html_dialog(NULL, url_moniker, NULL, extra_args, &v_result);
+  HRESULT hr = show_html_dialog(NULL,
+                                url_moniker,
+                                dialog_args.AsInput(),
+                                extra_args,
+                                &v_result);
   url_moniker->Release();
 
   if (v_result.vt == VT_I4)
@@ -163,8 +170,9 @@
                  reinterpret_cast<LPARAM>(ico));
 }
 
-EulaHTMLDialog::EulaHTMLDialog(const std::wstring& file) {
-  dialog_ = CreateNativeHTMLDialog(file);
+EulaHTMLDialog::EulaHTMLDialog(const std::wstring& file,
+                               const std::wstring& param) {
+  dialog_ = CreateNativeHTMLDialog(file, param);
 }
 
 EulaHTMLDialog::~EulaHTMLDialog() {