Reland r167487: Get full WebPluginInfo for the PDF plug-in before enabling it for print preview.

ui/base/win/window_impl.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 "ui/base/win/window_impl.h"
 
 #include <list>
 
 #include "base/debug/alias.h"
 #include "base/memory/singleton.h"
@@ skipping 26 matching lines @@
         icon(icon) {}
 
   // Compares two ClassInfos. Returns true if all members match.
   bool Equals(const ClassInfo& other) const {
     return (other.style == style && other.icon == icon);
   }
 };
 
 class ClassRegistrar {
  public:
   static ClassRegistrar* GetInstance() {

[Peter Kasting, 2012/11/15 18:53:50]

Nit: Would be good to de-inline this as well.

Also, Google style guide says static member functions go after
constructor/destructor.

[Bernhard Bauer, 2012/11/18 19:15:26]

Done.

-    return Singleton<ClassRegistrar>::get();
+    return Singleton<ClassRegistrar,
+                     LeakySingletonTraits<ClassRegistrar> >::get();
   }
 
   ~ClassRegistrar() {
-    for (RegisteredClasses::iterator i = registered_classes_.begin();
-         i != registered_classes_.end(); ++i) {
-      if (!UnregisterClass(MAKEINTATOM(i->atom), i->instance)) {
-        LOG(ERROR) << "Failed to unregister class " << i->name.c_str()
-                   << ". Error = " << GetLastError();

[cpu_(ooo_6.6-7.5), 2012/11/15 19:08:36]

So we are leaking the classes, I would be ok with this if you can assert we
create a bounded number of them. For example, tooltips and other transient
windows we make.

In regular windows programs you would only have a small set of window classes,
say 3 or 4, but our code is generic and complicated.

[Bernhard Bauer, 2012/11/15 23:26:30]

Hm, if I read http://crbug.com/113008 correct, we already fail to unregister
them either way, as there is still a window open when we try to.

In any case, I can run this with some logging tomorrow to get a feeling for the
number of classes.

[Bernhard Bauer, 2012/11/18 19:15:26]

Ok, I ran unit_tests and I got only one registration, for Chrome_WidgetWin_0.
During a regular browser run we might get more, but those weren't unregistered
before the end of the process anyway.

-      }
-    }
   }
 
   // Puts the name for the class matching |class_info| in |class_name|, creating
   // a new name if the class is not yet known.
   // Returns true if this class was already known, false otherwise.
   bool RetrieveClassName(const ClassInfo& class_info, std::wstring* name) {
     for (RegisteredClasses::const_iterator i = registered_classes_.begin();
          i != registered_classes_.end(); ++i) {
       if (class_info.Equals(i->info)) {
         name->assign(i->name);
         return true;
       }
     }
 
     name->assign(string16(WindowImpl::kBaseClassName) +
         base::IntToString16(registered_count_++));
     return false;
   }
 
   void RegisterClass(const ClassInfo& class_info,
-                     const std::wstring& name,
-                     ATOM atom,
-                     HMODULE instance) {
-    registered_classes_.push_back(
-        RegisteredClass(class_info, name, atom, instance));
+                     const std::wstring& name) {
+    registered_classes_.push_back(RegisteredClass(class_info, name));
   }
 
  private:
   // Represents a registered window class.
   struct RegisteredClass {
     RegisteredClass(const ClassInfo& info,
-                    const std::wstring& name,
-                    ATOM atom,
-                    HMODULE instance)
+                    const std::wstring& name)
         : info(info),
-          name(name),
-          atom(atom),
-          instance(instance) {
+          name(name) {
     }
 
     // Info used to create the class.
     ClassInfo info;
 
     // The name given to the window class.
     std::wstring name;
-
-    // The ATOM returned from registering the window class.

[cpu_(ooo_6.6-7.5), 2012/11/15 19:08:36]

I would rather keep the atom and ditch the name.

[Bernhard Bauer, 2012/11/15 23:26:30]

But we need the name to find classes by name, or am I missing something?

-    ATOM atom;
-
-    // The handle of the module containing the window procedure.
-    HMODULE instance;
   };
 
   ClassRegistrar() : registered_count_(0) { }
   friend struct DefaultSingletonTraits<ClassRegistrar>;
 
   typedef std::list<RegisteredClass> RegisteredClasses;
   RegisteredClasses registered_classes_;
 
   // Counter of how many classes have been registered so far.
   int registered_count_;
@@ skipping 142 matching lines @@
       NULL,
       reinterpret_cast<HBRUSH>(background + 1),
       NULL,
       icon,
       icon,
       &window_class);
   HMODULE instance = window_class.hInstance;
   ATOM atom = RegisterClassEx(&window_class);
   CHECK(atom) << GetLastError();
 
-  ClassRegistrar::GetInstance()->RegisterClass(
-      class_info, name, atom, instance);
+  ClassRegistrar::GetInstance()->RegisterClass(class_info, name);
 
   return name;
 }
 
 }  // namespace ui