Bug 24836 – Chromium's navigator.plugins.refresh(false) does nothing

• Component: Plug-ins
• Status: RESOLVED FIXED

[John Abd-El-Malek]

It's supposed to reload the plugin list, but not the page.  Chromium's implementation didn't reload the plugin list until it was first accessed.

[John Abd-El-Malek]

Created attachment 28955 [details]
Fix

[Eric Seidel (no email)]

Comment on attachment 28955 [details]
Fix

OK.  Looks sane.  You should document what that line is actually for with a comment.

I assume this has no layout test because the layout tests are in chromium's tree (and this doesn't affect any other platform?)

[John Abd-El-Malek]

Created attachment 28977 [details]
added comment

[John Abd-El-Malek]

Added comment.  Can you commit for me (I'm not a committer)?

This doesn't affect other ports.  The place to add a test would be in Chromium's ui tests.  I didn't add one yet because I would need to create a new NPAPI plugin just to test this scenario.  I'll look into doing it now.

[Darin Fisher (:fishd, Google)]

john, can't you utilize the layout tests for this?  it is best if webkit specific patches are tested with layout tests.  if that can't be done, then in the chromium world test_shell tests could be used.  if we have to resort to chromium ui tests, then we are far removed from the webkit code, which could mean that we only discover a regression once we update the webkit code used by chromium.

[John Abd-El-Malek]

To test this change, one would need two NPAPI plugins.  The first would have to move the second, get the list of plugins, then move it back and check that refresh(false) caused the list to be updated.  Chromium's two test NPAPI plugins are not in the WebKit tree so a layout test won't work.  I'll look into putting it into testshell_tests instead.

[John Abd-El-Malek]

I've added a test_shell_tests test for this.  Can someone commit this, because I can't check in the other change until this one lands or else the test will fail.

[John Abd-El-Malek]

Here's the test: http://codereview.chromium.org/42687

[Dmitry Titov]

Landed: http://trac.webkit.org/changeset/42052