Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(24)

Issue 196012: This changelist fixes some issues with the NPAPI WMP plugin work in Chrome. ... (Closed)

Created:
11 years, 3 months ago by jam
Modified:
11 years, 3 months ago
Reviewers:
ananta
CC:
chromium-reviews_googlegroups.com, darin (slow to review), brettw, jam, Ben Goodger (Google)
Visibility:
Public.

Description

This changelist fixes some issues with the NPAPI WMP plugin work in Chrome. The first is that we need to disable windowless mode since it doesn't work in the NPAPI plugin (Safari does this as well, and sites don't use windowless for Firefox). The second is to make UpdateGeometry message synchronous for WMP. The problem I saw was that while handling that message, the plugin might disaptch a NPObject Invoke method to play a video, which WMP doesn't expect and it leads to the video never playing. While touching these files, I made some small cleanup by reverting the change that made WebPluginProxy not have a WebPluginDelegateImpl pointer, which added a bunch of unnecessary methods to WebPluginDelegate. BUG=20259 TEST=use --no-activex and try playing the videos on http://www.nana10.co.il/Section/?SectionID=10847&sid=235 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25433

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -240 lines) Patch
M chrome/browser/plugin_process_host.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/plugin_process_host.cc View 4 chunks +7 lines, -6 lines 0 comments Download
M chrome/browser/plugin_service.cc View 2 chunks +1 line, -6 lines 0 comments Download
M chrome/browser/renderer_host/resource_message_filter.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/common/plugin_messages.h View 3 chunks +41 lines, -1 line 0 comments Download
M chrome/common/plugin_messages_internal.h View 1 chunk +7 lines, -9 lines 0 comments Download
M chrome/common/render_messages_internal.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/plugin/webplugin_delegate_stub.h View 4 chunks +4 lines, -8 lines 0 comments Download
M chrome/plugin/webplugin_delegate_stub.cc View 6 chunks +14 lines, -25 lines 0 comments Download
M chrome/plugin/webplugin_proxy.h View 3 chunks +3 lines, -5 lines 0 comments Download
M chrome/plugin/webplugin_proxy.cc View 2 chunks +1 line, -3 lines 0 comments Download
M chrome/renderer/render_view.cc View 3 chunks +3 lines, -4 lines 0 comments Download
M chrome/renderer/webplugin_delegate_proxy.h View 5 chunks +10 lines, -17 lines 0 comments Download
M chrome/renderer/webplugin_delegate_proxy.cc View 8 chunks +39 lines, -58 lines 0 comments Download
M webkit/default_plugin/plugins2.xml View 1 chunk +12 lines, -5 lines 0 comments Download
M webkit/glue/plugins/webplugin_delegate_impl.h View 3 chunks +23 lines, -4 lines 0 comments Download
M webkit/glue/plugins/webplugin_delegate_impl.cc View 1 4 chunks +23 lines, -8 lines 0 comments Download
M webkit/glue/plugins/webplugin_delegate_impl_mac.mm View 2 4 chunks +18 lines, -5 lines 0 comments Download
M webkit/glue/plugins/webplugin_delegate_impl_win.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M webkit/glue/webplugin_delegate.h View 4 chunks +5 lines, -31 lines 0 comments Download
M webkit/glue/webplugin_impl.h View 1 chunk +2 lines, -3 lines 0 comments Download
M webkit/glue/webplugin_impl.cc View 5 chunks +10 lines, -36 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
jam
11 years, 3 months ago (2009-09-03 22:06:20 UTC) #1
jam
11 years, 3 months ago (2009-09-04 05:47:30 UTC) #2
On Thu, Sep 3, 2009 at 10:37 PM, <ananta@chromium.org> wrote:

> LGTM
>
>
> http://codereview.chromium.org/196012/diff/1/20
> File chrome/renderer/webplugin_delegate_proxy.cc (right):
>
> http://codereview.chromium.org/196012/diff/1/20#newcode434
> Line 434: if (transport_store_.get()) {
> The original code had the following check as well.
> if (transport_store_.get() && background_store.get() {
>  blah blah
> }


> Is this not needed anymore?
>

I simplified the logic and moved it up a few lines.


>
> http://codereview.chromium.org/196012/diff/1/20#newcode448
> Line 448: msg = new PluginMsg_UpdateGeometrySync(instance_id_, param);
> Sad that the UpdateGeometry message needs to be sync :(


I think it's interesting that we never hit this with Flash or other plugins
before, but either way it's only sync for WMP.


>
>
> http://codereview.chromium.org/196012
>

Powered by Google App Engine
This is Rietveld 408576698