Moved the following IPC messages used by the chrome NPAPI plugin installer out of content

Moved the following IPC messages used by the chrome NPAPI plugin installer out of content
into Chrome.

1. PluginProcessHostMsg_GetPluginFinderUrl
2. PluginProcessHostMsg_MissingPluginStatus
3. PluginProcessHostMsg_DownloadUrl

These messages are prefixed with Chrome. Removed the InstallMissingPlugin and OnInstallMissingPlugin
handlers from the NPAPI plugin sources and from our webkit plugin implementation. The plugin infobar
no longer sends over an IPC message to initiate installation of the third party plugin. It sends over
a windows message which is handled in the plugin installer for Windows. This functionality is not
implemented for the mac and linux as before.

To display the plugin installation infobar the PluginProcessHostMsg_MissingPluginStatus message
sent by the plugin needs the routing id and the renderer process id. This information is now
passed along with the plugin instantiation parameters in NPP_New. These parameters are only read
by the default plugin.

This is a continuation of the fixes to ensure that IPC's don't span across content and chrome.

BUG=87335
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99062

[jam, 2011-08-31 17:34:42]

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
File chrome/browser/plugin_installer_infobar_delegate.cc (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
chrome/browser/plugin_installer_infobar_delegate.cc:56: ::SendMessageTimeout(
why send a message with a timeout of 50ms (which is unclear how you picked that
number) instead of posting it? before we were sending an async IPC anyways.

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
File chrome/browser/plugin_installer_infobar_delegate.h (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
chrome/browser/plugin_installer_infobar_delegate.h:17: explicit
PluginInstallerInfoBarDelegate(TabContents* tab_contents,
nit: explicit isn't needed anymore

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_observ...
File chrome/browser/plugin_observer.cc (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_observ...
chrome/browser/plugin_observer.cc:295: void
PluginObserver::OnMissingPluginStatus(int status,
why is this method now on PluginObserver, since it's only called by
ChromePluginMessageFilter? The former is for dispatching messages from the
renderer, while the latter is for messages from the plugin process. it seems a
bit odd to dispatch a message from the plugin process and then just call a
function on the renderer's filter to handle it

http://codereview.chromium.org/7812020/diff/6019/chrome/common/chrome_plugin_...
File chrome/common/chrome_plugin_messages.h (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/common/chrome_plugin_...
chrome/common/chrome_plugin_messages.h:30: int /* The plugin window handle */)
why send a window handle as an int instead of a gfx::NativeWindow?

http://codereview.chromium.org/7812020/diff/6019/chrome/default_plugin/plugin...
File chrome/default_plugin/plugin_main.h (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/default_plugin/plugin...
chrome/default_plugin/plugin_main.h:44: class PluginInstallerBase {
nit: this should go into its own file

http://codereview.chromium.org/7812020/diff/6019/chrome/default_plugin/plugin...
chrome/default_plugin/plugin_main.h:52: int renderer_process_id_;
why not just add getters for derived classes instead of exposing the member
variables?

http://codereview.chromium.org/7812020/diff/6019/content/common/content_const...
File content/common/content_constants.h (right):

http://codereview.chromium.org/7812020/diff/6019/content/common/content_const...
content/common/content_constants.h:33: // The render view and render process ids
associated with the plugin instance.
nit: this comment is a bit vague, since it's not clear that this is only for the
installer plugin

http://codereview.chromium.org/7812020/diff/6019/content/plugin/webplugin_del...
File content/plugin/webplugin_delegate_stub.cc (right):

http://codereview.chromium.org/7812020/diff/6019/content/plugin/webplugin_del...
content/plugin/webplugin_delegate_stub.cc:186: // Add the renderer process id
and Render view routing id to the list of
we shouldn't have to expose this information to all plugins. also, who knows
which plugins break if they get unexpected args..

[ananta, 2011-08-31 20:18:14]

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
File chrome/browser/plugin_installer_infobar_delegate.cc (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
chrome/browser/plugin_installer_infobar_delegate.cc:56: ::SendMessageTimeout(
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> why send a message with a timeout of 50ms (which is unclear how you picked
that
> number) instead of posting it? before we were sending an async IPC anyways.
Done. Dumb error.

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
File chrome/browser/plugin_installer_infobar_delegate.h (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_instal...
chrome/browser/plugin_installer_infobar_delegate.h:17: explicit
PluginInstallerInfoBarDelegate(TabContents* tab_contents,
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> nit: explicit isn't needed anymore

Done.

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_observ...
File chrome/browser/plugin_observer.cc (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/browser/plugin_observ...
chrome/browser/plugin_observer.cc:295: void
PluginObserver::OnMissingPluginStatus(int status,
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> why is this method now on PluginObserver, since it's only called by
> ChromePluginMessageFilter? The former is for dispatching messages from the
> renderer, while the latter is for messages from the plugin process. it seems a
> bit odd to dispatch a message from the plugin process and then just call a
> function on the renderer's filter to handle it

Moved the handling to ChromePluginMessageFilter.

http://codereview.chromium.org/7812020/diff/6019/chrome/common/chrome_plugin_...
File chrome/common/chrome_plugin_messages.h (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/common/chrome_plugin_...
chrome/common/chrome_plugin_messages.h:30: int /* The plugin window handle */)
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> why send a window handle as an int instead of a gfx::NativeWindow?

Done.

http://codereview.chromium.org/7812020/diff/6019/chrome/default_plugin/plugin...
File chrome/default_plugin/plugin_main.h (right):

http://codereview.chromium.org/7812020/diff/6019/chrome/default_plugin/plugin...
chrome/default_plugin/plugin_main.h:44: class PluginInstallerBase {
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> nit: this should go into its own file

Done.

http://codereview.chromium.org/7812020/diff/6019/chrome/default_plugin/plugin...
chrome/default_plugin/plugin_main.h:52: int renderer_process_id_;
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> why not just add getters for derived classes instead of exposing the member
> variables?

Done.

http://codereview.chromium.org/7812020/diff/6019/content/common/content_const...
File content/common/content_constants.h (right):

http://codereview.chromium.org/7812020/diff/6019/content/common/content_const...
content/common/content_constants.h:33: // The render view and render process ids
associated with the plugin instance.
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> nit: this comment is a bit vague, since it's not clear that this is only for
the
> installer plugin

Done.

http://codereview.chromium.org/7812020/diff/6019/content/plugin/webplugin_del...
File content/plugin/webplugin_delegate_stub.cc (right):

http://codereview.chromium.org/7812020/diff/6019/content/plugin/webplugin_del...
content/plugin/webplugin_delegate_stub.cc:186: // Add the renderer process id
and Render view routing id to the list of
On 2011/08/31 17:34:42, John Abd-El-Malek wrote:
> we shouldn't have to expose this information to all plugins. also, who knows
> which plugins break if they get unexpected args..

Done.

[jam, 2011-08-31 20:23:57]

lgtm

http://codereview.chromium.org/7812020/diff/8051/chrome/browser/plugin_observ...
File chrome/browser/plugin_observer.h (right):

http://codereview.chromium.org/7812020/diff/8051/chrome/browser/plugin_observ...
chrome/browser/plugin_observer.h:10: #include "ui/gfx/native_widget_types.h"
nit: is this needed?

http://codereview.chromium.org/7812020/diff/8051/content/common/content_const...
File content/common/content_constants.h (right):

http://codereview.chromium.org/7812020/diff/8051/content/common/content_const...
content/common/content_constants.h:33: // The render view and render process id
associated with the installer plugin
nit: perhaps just default_plugin to match the other terminology in content?

[ananta, 2011-08-31 20:44:23]

http://codereview.chromium.org/7812020/diff/8051/chrome/browser/plugin_observ...
File chrome/browser/plugin_observer.h (right):

http://codereview.chromium.org/7812020/diff/8051/chrome/browser/plugin_observ...
chrome/browser/plugin_observer.h:10: #include "ui/gfx/native_widget_types.h"
On 2011/08/31 20:23:57, John Abd-El-Malek wrote:
> nit: is this needed?

Done.

http://codereview.chromium.org/7812020/diff/8051/content/common/content_const...
File content/common/content_constants.h (right):

http://codereview.chromium.org/7812020/diff/8051/content/common/content_const...
content/common/content_constants.h:33: // The render view and render process id
associated with the installer plugin
On 2011/08/31 20:23:57, John Abd-El-Malek wrote:
> nit: perhaps just default_plugin to match the other terminology in content?

Done.