HandleDownloadResponse being called multiple times on JS window.open call

cloudtrends / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded

1 stars 1 forks source link

HandleDownloadResponse being called multiple times on JS window.open call #215

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago

What steps will reproduce the problem?
1. Use the JavaScript window.open function to try to download a file through a 
popup window.
2. Close the popup window that opens after download is initiated
3. Repeat step 1 and HandleDownloadResponse should be called twice.

What is the expected output? What do you see instead?

HandleDownloadResponse should be called once every time.

What version of the product are you using? On what operating system?

CEF revision 181 built locally on Windows XP.

Please provide any additional information below.

This is the example HTML file I use for testing this:

<html>
   <body>
      <p>
         <a onclick="window.open('http://www.microsoft.com/investor/Downloads/Earnings%20and%20Financials/Earnings%20Releases/PressRelease/FY11/Q1/Q1-FY11Slides.pptx');">window.open(download)</a>
      </p>
   </body>
</html>

Original issue reported on code.google.com by nesf...@gmail.com on 30 Mar 2011 at 9:17

GoogleCodeExporter commented 9 years ago

Checking out issue 204 
(http://code.google.com/p/chromiumembedded/issues/detail?id=204) as they might 
be related / duplicates.

Original comment by nesf...@gmail.com on 30 Mar 2011 at 10:28

GoogleCodeExporter commented 9 years ago

The fix for issue 204 is not applicable to this one.

Original comment by nesf...@gmail.com on 31 Mar 2011 at 5:33

GoogleCodeExporter commented 9 years ago

In my testing with revision 212 HandleDownloadResponse is called once each time 
the link is clicked. You can verify this by using cefclient at revision 212 and 
setting a breakpoint in ClientHandler::HandleDownloadResponse.

Original comment by magreenb...@gmail.com on 4 Apr 2011 at 2:03

Changed state: WontFix

GoogleCodeExporter commented 9 years ago

I am able to reproduce this consistently with the r212 binaries:

1. Launch cefclient.exe from the r212 binary distribution
2. Load a file containing the html in the original description
3. Click on "window.open(download)"
4. A message box appears saying the download finished.  "My Documents" contains 
a new 984KB file named "Q1-FY11Slides.pptx"
5. Close the message box and popup window
6. Click on "window.open(download)"
7. Two message boxes appear saying a download finished.  My Documents contains 
two new files: a 16KB file "Q1-FY11Slides (1).pptx" and a 984KB file 
"Q1-FY11Slides (2).pptx"

Each subsequent repeat of steps 5 and 6 results in 2 downloads: one that is the 
expected size, one that is 16KB.  This doesn't happen with some other URLs that 
I've tried, nor the first time in this example.  I believe caching plays a part

Original comment by mikeyk...@gmail.com on 4 May 2011 at 8:36

GoogleCodeExporter commented 9 years ago

I should mention I was using the Release cefclient.exe.  I was not able to 
reproduce with the Debug version with the limited testing that I did

Original comment by mikeyk...@gmail.com on 5 May 2011 at 5:34

GoogleCodeExporter commented 9 years ago

I have a patch for this issue.  It prevents us from loading the URL when we 
create a popup window.  To see why this change is necessary, we can look at 
WebCore::DOMWindow::createWindow.  For a new popup, this code strips down to:

Frame* newFrame = WebCore::createWindow(...);
newFrame->loader()->changeLocation(...);

Both of these lines were triggering a URL request in CEF (See the attached 
callstacks run1.txt and run2.txt.)  Since the WebCore function sets the new 
location, we shouldn't be loading a URL during creation.

This wasn't always a problem.  Before r162 we didn't have a URL when creating 
the popup, so UIT_LoadURL was not called at popup creation time.  As expected, 
I was not able to reproduce the bug with the r158 binaries, but I was able to 
reproduce with r181 and higher.

Original comment by mikeyk...@gmail.com on 5 May 2011 at 7:41

Attachments:

GoogleCodeExporter commented 9 years ago

@comment#6: Thank you for the analysis. Unfortunately this means that we can't 
change the |url| value in HandleBeforeCreated when creating a popup window. We 
should probably make the |url| parameter read-only.

Original comment by magreenb...@gmail.com on 5 May 2011 at 7:56

Changed state: Accepted

GoogleCodeExporter commented 9 years ago

Fixed in revision 227.

Original comment by magreenb...@gmail.com on 10 May 2011 at 1:55

Changed state: Fixed