Right now, file drag out isn't very well supported. We have downloadurl and the default drag handler for images, but we don't actually support image dragout using the specced method.
Created attachment 133417 [details] Patch
Comment on attachment 133417 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=133417&action=review r- for no test. Please add one to ManualTests or create a layout test. > Source/WebCore/ChangeLog:8 > + No new tests. Manually tested that file drag and drop doesnot crash chrome. Please create a layout test or at least make a manual test and put it in ManualTests. There are a few other drag&drop tests in that directory already. > Source/WebCore/platform/chromium/DataTransferItemListChromium.cpp:123 > + // Note that passing file to createFromFile below transfers ownership of > + // it, so file is 0 after this call. > + m_itemList.append(DataTransferItemChromium::createFromFile(file)); Are you sure about this comment? |file| is a ref counted pointer so createFromFile shouldn't delete anything. Is the order that we add items important here? Daniel would know.
(In reply to comment #2) > (From update of attachment 133417 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133417&action=review > > r- for no test. Please add one to ManualTests or create a layout test. Daniel, are you taking this over now that you're back in the office? > > Source/WebCore/platform/chromium/DataTransferItemListChromium.cpp:123 > > + // Note that passing file to createFromFile below transfers ownership of > > + // it, so file is 0 after this call. > > + m_itemList.append(DataTransferItemChromium::createFromFile(file)); > > Are you sure about this comment? |file| is a ref counted pointer so createFromFile shouldn't delete anything. Is the order that we add items important here? Daniel would know. Moving this definitely fixed the crash I saw: crbug.com/119591 I'm new to these WebKit types, but http://www.webkit.org/coding/RefPtr.html says that assigning a PassRefPtr to a RefPtr clears the original pointer, and inside createFromFile we have: item->m_file = file; Transferring ownership of the File to the DataTransferItemChromium. I assumed the order of the items was irrelevant, but Daniel is the expert.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 133417 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=133417&action=review > > > Source/WebCore/platform/chromium/DataTransferItemListChromium.cpp:123 > > > + // Note that passing file to createFromFile below transfers ownership of > > > + // it, so file is 0 after this call. > > > + m_itemList.append(DataTransferItemChromium::createFromFile(file)); > > > > Are you sure about this comment? |file| is a ref counted pointer so createFromFile shouldn't delete anything. Is the order that we add items important here? Daniel would know. > > Moving this definitely fixed the crash I saw: crbug.com/119591 > I'm new to these WebKit types, but http://www.webkit.org/coding/RefPtr.html says that assigning a PassRefPtr to a RefPtr clears the original pointer, and inside createFromFile we have: > item->m_file = file; > Transferring ownership of the File to the DataTransferItemChromium. createFromFile gets a copy of PassRefPtr. The original copy in DataTransferItemListChromium is unmodified.
Created attachment 133568 [details] Patch
Comment on attachment 133568 [details] Patch Will this test pass on other ports or should it be in the Skipped files?
Committed r111919: <http://trac.webkit.org/changeset/111919>