Closed GoogleCodeExporter closed 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:
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
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
Added issue #637 for CefDownloadManagerDelegate::ChooseDownloadPath support on
Linux.
Original comment by magreenb...@gmail.com
on 11 Jun 2012 at 4:08
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
Original issue reported on code.google.com by
corey.lu...@gmail.com
on 8 Jun 2012 at 3:41