Show a new placeholder for PDF's when plugins are disabled.

chrome/renderer/plugins/pdf_plugin_placeholder.h

+// Copyright 2017 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.
+
+#ifndef CHROME_RENDERER_PLUGINS_PDF_PLUGIN_PLACEHOLDER_H_
+#define CHROME_RENDERER_PLUGINS_PDF_PLUGIN_PLACEHOLDER_H_
+
+#include "components/plugins/renderer/plugin_placeholder.h"
+
+// Creates a placeholder that allows users to click to download a PDF for when

[Lei Zhang, 2017/06/30 19:35:21]

nit: The class dosen't create a placeholder. It is the placeholder, right?

[amberwon, 2017/06/30 21:59:22]

Done.

+// plugins are disabled and the PDF fails to load.
+// TODO(amberwon): Flesh out the class more to download an embedded PDF when the
+// PDF plugin is disabled or unavailable.
+class PDFPluginPlaceholder final : public plugins::PluginPlaceholderBase,
+                                   public gin::Wrappable<PDFPluginPlaceholder> {
+ public:
+  static gin::WrapperInfo kWrapperInfo;

[Lei Zhang, 2017/06/30 19:35:21]

Does this need the const keyword?

[amberwon, 2017/06/30 21:59:22]

No, in the Wrappable class, kWrapperInfo is passed in a function call that
requires a non-const value.

[Lei Zhang, 2017/06/30 22:45:11]

Ok, I guess this is a weird Gin thing. Let's leave it be.

[amberwon, 2017/06/30 23:11:12]

Acknowledged.

+
+  static PDFPluginPlaceholder* CreatePDFPlaceholder(

[Lei Zhang, 2017/06/30 19:35:21]

Can you add a comment to explain the ownership of the returned pointer?

[amberwon, 2017/06/30 21:59:22]

Done.

+      content::RenderFrame* render_frame,
+      const blink::WebPluginParams& params);
+
+ private:
+  PDFPluginPlaceholder(content::RenderFrame* render_frame,
+                       const blink::WebPluginParams& params,
+                       const std::string& html_data);
+  ~PDFPluginPlaceholder() override;

[Lei Zhang, 2017/06/30 19:35:21]

We usually don't bother marking classes / methods as final, unless there's some
specific reason. If we are going to use final for a class, then all of its
overrides should be final too.

[amberwon, 2017/06/30 21:59:22]

Done.

[Lei Zhang, 2017/06/30 22:45:11]

So they are now consistent, but how about we make them finals instead of
overrides?

[amberwon, 2017/06/30 23:11:12]

Done.

+
+  // WebViewPlugin::Delegate methods:
+  v8::Local<v8::Value> GetV8Handle(v8::Isolate* isolate) final;
+};
+
+#endif  // CHROME_RENDERER_PLUGINS_PDF_PLUGIN_PLACEHOLDER_H_