Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

[Android] Fix XWALK unable to support download behavior #2590

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[Android] Fix XWALK unable to support download behavior #2590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion runtime/android/core_internal/src/org/xwalk/core/internal/XWalkContentsClientBridge.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,12 +373,16 @@ public void onDownloadStart(String url,
String contentDisposition,
String mimeType,
long contentLength) {
if (mDownloadListener != null) {
mDownloadListener.onDownloadStart(url, userAgent, contentDisposition,
mimeType, contentLength);
}
}

@Override
public boolean onCreateWindow(boolean isDialog, boolean isUserGesture) {
if (isDialog) return false;

XWalkUIClientInternal.InitiateByInternal initiator =
XWalkUIClientInternal.InitiateByInternal.BY_JAVASCRIPT;
if (isUserGesture) {
Expand Down
17 changes: 15 additions & 2 deletions runtime/browser/android/xwalk_contents_io_thread_client_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,21 @@ void RfhToIoThreadClientMap::Set(pair<int, int> rfh_id,
bool RfhToIoThreadClientMap::Get(
pair<int, int> rfh_id, IoThreadClientData* client) {
base::AutoLock lock(map_lock_);
RenderFrameHostToIoThreadClientType::iterator iterator =
rfh_to_io_thread_client_.find(rfh_id);
RenderFrameHostToIoThreadClientType::iterator iterator;

if (rfh_id.second != MSG_ROUTING_NONE) {
iterator = rfh_to_io_thread_client_.find(rfh_id);
} else {
// Content use render_frame_id= MSG_ROUTING_NONE for download request
// So just find the matched process_id
iterator = rfh_to_io_thread_client_.begin();
while (iterator != rfh_to_io_thread_client_.end()) {
if (iterator->first.first == rfh_id.first)
break;
iterator++;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change coming from upstream? Any reference commit from upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wang16 The change is not from upstream, I debugged and found the solution by myself.
The story is Content is using MSG_ROUTING_NONE for render_frame_id. Previous implementation fails to find IoThreadClientData for it. Current change will ignore MSG_ROUTING_NONE..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And It seems both content shell and Android Webview falls to download by my test, so I am wondering where to find reference

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So is this an upstream bug? will this change apply to upstream as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to investigate. But I think we can commit in XWALK first, as in the bug, customer is waiting for the fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wang16
https://code.google.com/p/chromium/issues/detail?id=432414&thanks=432414&ts=1415773645
This is the bug link, seems no response yet.

There are 2 implementions from upstream, while Crosswalk's implementation is almost the same with Androidwebview for the part which support download. AndroidWebview does not support this download too...The other implementation is from Chrome Shell, which is very different from XWalk's

}
}

if (iterator == rfh_to_io_thread_client_.end())
return false;

Expand Down
2 changes: 1 addition & 1 deletion runtime/browser/runtime_download_manager_delegate.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void RuntimeDownloadManagerDelegate::ChooseDownloadPath(
if (GetSaveFileName(&save_as))
result = base::FilePath(std::wstring(save_as.lpstrFile));
#else
NOTIMPLEMENTED();
result = suggested_path;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will change the Crosswalk behavior in many other platforms beside the Android.

#endif

callback.Run(result, content::DownloadItem::TARGET_DISPOSITION_PROMPT,
Expand Down
2 changes: 0 additions & 2 deletions runtime/browser/runtime_resource_dispatcher_host_delegate_android.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,6 @@ void RuntimeResourceDispatcherHostDelegateAndroid::DownloadStarting(
response_headers->GetMimeType(&mime_type);
}

request->Cancel();

const content::ResourceRequestInfo* request_info =
content::ResourceRequestInfo::ForRequest(request);

Expand Down