Bug 24836

Summary: Chromium's navigator.plugins.refresh(false) does nothing
Product: WebKit Reporter: John Abd-El-Malek <jam>
Component: Plug-insAssignee: Dmitry Titov <dimich>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Fix
eric: review+
added comment none

Description John Abd-El-Malek 2009-03-25 17:24:33 PDT
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.
Comment 1 John Abd-El-Malek 2009-03-25 17:27:41 PDT
Created attachment 28955 [details]
Fix
Comment 2 Eric Seidel (no email) 2009-03-26 10:56:14 PDT
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?)
Comment 3 John Abd-El-Malek 2009-03-26 12:30:39 PDT
Created attachment 28977 [details]
added comment
Comment 4 John Abd-El-Malek 2009-03-26 12:33:26 PDT
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.
Comment 5 Darin Fisher (:fishd, Google) 2009-03-26 12:47:57 PDT
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.
Comment 6 John Abd-El-Malek 2009-03-26 13:15:03 PDT
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.
Comment 7 John Abd-El-Malek 2009-03-26 20:22:51 PDT
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.
Comment 8 John Abd-El-Malek 2009-03-26 20:23:08 PDT
Here's the test: http://codereview.chromium.org/42687
Comment 9 Dmitry Titov 2009-03-27 15:35:36 PDT
Landed: http://trac.webkit.org/changeset/42052