search

pacificIT / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
0 stars 1 forks source link

CEF3: CefDownloadManagerDelegate::ChooseDownloadPath doesn't have OSX implementation. #634

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
CefDownloadManagerDelegate::ChooseDownloadPath currently only supports an 
OS_WIN implementation, as such, downloads on OSX are not functioning.

Original issue reported on code.google.com by corey.lu...@gmail.com on 8 Jun 2012 at 3:41

GoogleCodeExporter commented 9 years ago 
Patch attached.  Refactored to support platform-specific implementation.  

Included in the patch (but unrelated to the bug, but benign all the same), is 
inclusion of NOTIMPLEMENTED() macro for CefBrowserHostImpl::RunFileChooser so 
it's easier to detect when debugging.  Will post patch for this to the 
corresponding bug as well.

Original comment by corey.lu...@gmail.com on 8 Jun 2012 at 3:44

Attachments:

GoogleCodeExporter commented 9 years ago 
Overall, looks good. A few style-related comments:

http://code.google.com/p/chromiumembedded/issues/attachmentText?id=634&aid=63400
01000&name=patch.txt&token=05LAnE3UUvisWkeSjuXRHdpMgxQ%3A1339170599547#14
+#import <Cocoa/Cocoa.h>

System headers should come after the source file's header 
(download_manager_delegate.h) and before other headers.

http://code.google.com/p/chromiumembedded/issues/attachmentText?id=634&aid=63400
01000&name=patch.txt&token=05LAnE3UUvisWkeSjuXRHdpMgxQ%3A1339170599547#106
+#include "libcef/browser/download_manager_delegate.h"

Same as above.

http://code.google.com/p/chromiumembedded/issues/attachmentText?id=634&aid=63400
01000&name=patch.txt&token=05LAnE3UUvisWkeSjuXRHdpMgxQ%3A1339170599547#107
+#include "content/public/browser/web_contents.h"
+#include "base/string_util.h"

Chromium headers should be in alphabetical order.

http://code.google.com/p/chromiumembedded/issues/attachmentText?id=634&aid=63400
01000&name=patch.txt&token=05LAnE3UUvisWkeSjuXRHdpMgxQ%3A1339170599547#139
Index: libcef/browser/browser_host_impl.cc

Please don't include unrelated changes.

Original comment by magreenb...@gmail.com on 8 Jun 2012 at 3:56

GoogleCodeExporter commented 9 years ago 
Updated...

Original comment by corey.lu...@gmail.com on 8 Jun 2012 at 4:16

Attachments:

GoogleCodeExporter commented 9 years ago 
Per the content API spec we need to implement 'AddItemToPersistentStore' in the 
delegate or DownloadManager will delete all uncompleted and un-persisted 
downloads. Using '0' for now for db_handle since CEF3 doesn't maintain a 
history db.

void 
CefDownloadManagerDelegate::AddItemToPersistentStore(content::DownloadItem* 
item) {
  download_manager_->OnItemAddedToPersistentStore(item->GetId(), 0);
}

Original comment by magreenb...@gmail.com on 11 Jun 2012 at 3:59

GoogleCodeExporter commented 9 years ago 
Added issue #637 for CefDownloadManagerDelegate::ChooseDownloadPath support on 
Linux.

Original comment by magreenb...@gmail.com on 11 Jun 2012 at 4:08

GoogleCodeExporter commented 9 years ago 
Fixed in revision 682:

- Mac: Add CefDownloadManagerDelegate::ChooseDownloadPath implementation.
- Persist downloaded files after CEF exits.
- Shutdown the DownloadManager when CEF exits.
- Don't show an error message when downloading files with cefclient.

Original comment by magreenb...@gmail.com on 11 Jun 2012 at 5:35