Index: android_webview/browser/aw_permission_manager.cc |
diff --git a/android_webview/browser/aw_permission_manager.cc b/android_webview/browser/aw_permission_manager.cc |
index a428250f940152f3d242bc52f2892e37674f6cf7..424166644df4de27a5f3a176a79e4516ea52692a 100644 |
--- a/android_webview/browser/aw_permission_manager.cc |
+++ b/android_webview/browser/aw_permission_manager.cc |
@@ -18,8 +18,22 @@ |
using blink::mojom::PermissionStatus; |
using content::PermissionType; |
+using RequestPermissionsCallback = |
+ base::Callback<void(const std::vector<PermissionStatus>&)>; |
+ |
namespace android_webview { |
+namespace { |
+ |
+void PermissionRequestResponseCallbackWrapper( |
+ const base::Callback<void(PermissionStatus)>& callback, |
+ const std::vector<PermissionStatus>& vector) { |
+ DCHECK_EQ(vector.size(), 1ul); |
+ callback.Run(vector[0]); |
+} |
+ |
+} // namespace |
+ |
class LastRequestResultCache { |
public: |
LastRequestResultCache() = default; |
@@ -131,29 +145,82 @@ class LastRequestResultCache { |
DISALLOW_COPY_AND_ASSIGN(LastRequestResultCache); |
}; |
-struct AwPermissionManager::PendingRequest { |
+class AwPermissionManager::PendingRequest { |
Tobias Sargeant
2016/07/12 12:43:23
I understand the desire to clean up, but this clas
Takashi Toyoshima
2016/07/13 06:00:10
My intention was to minimize difference between th
|
public: |
- PendingRequest(PermissionType permission, |
+ PendingRequest(content::RenderFrameHost* render_frame_host, |
+ const std::vector<PermissionType> permissions, |
GURL requesting_origin, |
GURL embedding_origin, |
- content::RenderFrameHost* render_frame_host, |
- const base::Callback<void(PermissionStatus)>& callback) |
- : permission(permission), |
- requesting_origin(requesting_origin), |
- embedding_origin(embedding_origin), |
- render_process_id(render_frame_host->GetProcess()->GetID()), |
- render_frame_id(render_frame_host->GetRoutingID()), |
- callback(callback) { |
- } |
+ const RequestPermissionsCallback callback) |
+ : render_process_id_(render_frame_host->GetProcess()->GetID()), |
+ render_frame_id_(render_frame_host->GetRoutingID()), |
+ callback_(callback), |
+ permissions_(permissions), |
+ results_(permissions.size(), PermissionStatus::DENIED), |
+ remaining_results_(permissions.size()), |
+ requesting_origin_(requesting_origin), |
+ embedding_origin_(embedding_origin) {} |
~PendingRequest() = default; |
- PermissionType permission; |
- GURL requesting_origin; |
- GURL embedding_origin; |
- int render_process_id; |
- int render_frame_id; |
- base::Callback<void(PermissionStatus)> callback; |
+ void SetPermissionStatus(int index, PermissionStatus status) { |
+ DCHECK(!IsComplete()); |
+ |
+ results_[index] = status; |
+ --remaining_results_; |
+ } |
+ |
+ void SetPermissionStatus(PermissionType type, PermissionStatus status) { |
+ for (size_t i = 0; i < permissions_.size(); ++i) { |
+ if (permissions_[i] == type) { |
+ results_[i] = status; |
Tobias Sargeant
2016/07/12 12:43:23
Call SetPermissionStatus(i, status) here.
Takashi Toyoshima
2016/07/13 06:00:10
Done.
|
+ --remaining_results_; |
+ } |
+ } |
+ } |
+ |
+ bool HasPermissionType(PermissionType type) { |
+ for (auto permission : permissions_) { |
+ if (permission == type) |
+ return true; |
+ } |
+ return false; |
+ } |
+ |
+ bool IsComplete() const { |
+ return remaining_results_ == 0; |
+ } |
+ |
+ int render_process_id() const { return render_process_id_; } |
+ int render_frame_id() const { return render_frame_id_; } |
+ |
+ const RequestPermissionsCallback callback() const { return callback_; } |
+ |
+ std::vector<PermissionType> permissions() const { |
+ return permissions_; |
+ } |
+ |
+ std::vector<PermissionStatus> results() const { |
+ return results_; |
+ } |
+ |
+ const GURL& requesting_origin() const { |
+ return requesting_origin_; |
+ } |
+ |
+ const GURL& embedding_origin() const { |
+ return embedding_origin_; |
+ } |
+ |
+ private: |
+ int render_process_id_; |
+ int render_frame_id_; |
+ RequestPermissionsCallback callback_; |
+ std::vector<PermissionType> permissions_; |
+ std::vector<PermissionStatus> results_; |
+ size_t remaining_results_; |
+ GURL requesting_origin_; |
+ GURL embedding_origin_; |
}; |
AwPermissionManager::AwPermissionManager() |
@@ -163,6 +230,10 @@ AwPermissionManager::AwPermissionManager() |
} |
AwPermissionManager::~AwPermissionManager() { |
+ for (PendingRequestsMap::Iterator<PendingRequest> it(&pending_requests_); |
Takashi Toyoshima
2016/07/13 06:00:11
Should I call CancelPermissionRequest too?
|
+ !it.IsAtEnd(); it.Advance()) { |
+ it.GetCurrentValue()->callback().Run(it.GetCurrentValue()->results()); |
+ } |
} |
int AwPermissionManager::RequestPermission( |
@@ -171,147 +242,150 @@ int AwPermissionManager::RequestPermission( |
const GURL& requesting_origin, |
bool user_gesture, |
const base::Callback<void(PermissionStatus)>& callback) { |
+ return RequestPermissions( |
+ std::vector<PermissionType>(1, permission), |
+ render_frame_host, |
+ requesting_origin, |
+ user_gesture, |
+ base::Bind(&PermissionRequestResponseCallbackWrapper, callback)); |
+} |
+ |
+int AwPermissionManager::RequestPermissions( |
+ const std::vector<PermissionType>& permissions, |
+ content::RenderFrameHost* render_frame_host, |
+ const GURL& requesting_origin, |
+ bool user_gesture, |
+ const base::Callback<void( |
+ const std::vector<PermissionStatus>&)>& callback) { |
+ if (permissions.empty()) { |
+ callback.Run(std::vector<PermissionStatus>()); |
+ return kNoPendingOperation; |
+ } |
+ |
int render_process_id = render_frame_host->GetProcess()->GetID(); |
int render_frame_id = render_frame_host->GetRoutingID(); |
AwBrowserPermissionRequestDelegate* delegate = |
AwBrowserPermissionRequestDelegate::FromID(render_process_id, |
render_frame_id); |
if (!delegate) { |
- DVLOG(0) << "Dropping permission request for " |
- << static_cast<int>(permission); |
- callback.Run(PermissionStatus::DENIED); |
+ DVLOG(0) << "Dropping permissions request"; |
+ callback.Run(std::vector<PermissionStatus>( |
+ permissions.size(), PermissionStatus::DENIED)); |
return kNoPendingOperation; |
} |
- // Do not delegate any requests which are already pending. |
- bool should_delegate_request = true; |
- for (PendingRequestsMap::Iterator<PendingRequest> it(&pending_requests_); |
- !it.IsAtEnd(); it.Advance()) { |
- if (permission == it.GetCurrentValue()->permission) { |
- should_delegate_request = false; |
- break; |
- } |
- } |
- |
const GURL& embedding_origin = |
content::WebContents::FromRenderFrameHost(render_frame_host) |
->GetLastCommittedURL().GetOrigin(); |
- int request_id = kNoPendingOperation; |
- switch (permission) { |
- case PermissionType::GEOLOCATION: |
- request_id = pending_requests_.Add(new PendingRequest( |
- permission, requesting_origin, |
- embedding_origin, render_frame_host, |
- callback)); |
- if (should_delegate_request) { |
+ PendingRequest* pending_request = new PendingRequest( |
+ render_frame_host, |
+ permissions, |
+ requesting_origin, |
+ embedding_origin, |
+ callback); |
+ std::vector<bool> should_delegate_requests = |
+ std::vector<bool>(permissions.size()); |
+ for (size_t i = 0; i < permissions.size(); ++i) { |
+ bool should_delegate_request = true; |
+ for (PendingRequestsMap::Iterator<PendingRequest> it(&pending_requests_); |
+ !it.IsAtEnd(); it.Advance()) { |
+ if (it.GetCurrentValue()->HasPermissionType(permissions[i])) { |
+ // TODO(toyoshim): Shall we check to match requesting_origin too? |
+ should_delegate_request = false; |
+ break; |
+ } |
+ } |
+ should_delegate_requests[i] = should_delegate_request; |
+ } |
+ |
+ int request_id = pending_requests_.Add(pending_request); |
+ |
+ for (size_t i = 0; i < permissions.size(); ++i) { |
+ if (!should_delegate_requests[i]) |
+ continue; |
+ |
+ switch (permissions[i]) { |
+ case PermissionType::GEOLOCATION: |
delegate->RequestGeolocationPermission( |
requesting_origin, |
base::Bind(&OnRequestResponse, |
- weak_ptr_factory_.GetWeakPtr(), request_id, |
- callback)); |
- } |
- break; |
- case PermissionType::PROTECTED_MEDIA_IDENTIFIER: |
- request_id = pending_requests_.Add(new PendingRequest( |
- permission, requesting_origin, |
- embedding_origin, render_frame_host, |
- callback)); |
- if (should_delegate_request) { |
+ weak_ptr_factory_.GetWeakPtr(), request_id, i)); |
+ break; |
+ case PermissionType::PROTECTED_MEDIA_IDENTIFIER: |
delegate->RequestProtectedMediaIdentifierPermission( |
requesting_origin, |
base::Bind(&OnRequestResponse, |
- weak_ptr_factory_.GetWeakPtr(), request_id, |
- callback)); |
- } |
- break; |
- case PermissionType::MIDI_SYSEX: |
- request_id = pending_requests_.Add(new PendingRequest( |
- permission, requesting_origin, |
- embedding_origin, render_frame_host, |
- callback)); |
- if (should_delegate_request) { |
+ weak_ptr_factory_.GetWeakPtr(), request_id, i)); |
+ break; |
+ case PermissionType::MIDI_SYSEX: |
delegate->RequestMIDISysexPermission( |
requesting_origin, |
base::Bind(&OnRequestResponse, |
- weak_ptr_factory_.GetWeakPtr(), request_id, |
- callback)); |
- } |
- break; |
- case PermissionType::AUDIO_CAPTURE: |
- case PermissionType::VIDEO_CAPTURE: |
- case PermissionType::NOTIFICATIONS: |
- case PermissionType::PUSH_MESSAGING: |
- case PermissionType::DURABLE_STORAGE: |
- case PermissionType::BACKGROUND_SYNC: |
- NOTIMPLEMENTED() << "RequestPermission is not implemented for " |
- << static_cast<int>(permission); |
- callback.Run(PermissionStatus::DENIED); |
- break; |
- case PermissionType::MIDI: |
- callback.Run(PermissionStatus::GRANTED); |
- break; |
- case PermissionType::NUM: |
- NOTREACHED() << "PermissionType::NUM was not expected here."; |
- callback.Run(PermissionStatus::DENIED); |
- break; |
+ weak_ptr_factory_.GetWeakPtr(), request_id, i)); |
+ break; |
+ case PermissionType::AUDIO_CAPTURE: |
+ case PermissionType::VIDEO_CAPTURE: |
+ case PermissionType::NOTIFICATIONS: |
+ case PermissionType::PUSH_MESSAGING: |
+ case PermissionType::DURABLE_STORAGE: |
+ case PermissionType::BACKGROUND_SYNC: |
Tobias Sargeant
2016/07/12 12:43:23
After your change, |callback| will not be called u
Takashi Toyoshima
2016/07/13 06:00:10
Thank you for catching this bug.
request_id is ne
|
+ NOTIMPLEMENTED() << "RequestPermissions is not implemented for " |
+ << static_cast<int>(permissions[i]); |
+ pending_request->SetPermissionStatus(i, PermissionStatus::DENIED); |
+ break; |
+ case PermissionType::MIDI: |
+ pending_request->SetPermissionStatus(i, PermissionStatus::GRANTED); |
+ break; |
+ case PermissionType::NUM: |
+ NOTREACHED() << "PermissionType::NUM was not expected here."; |
+ pending_request->SetPermissionStatus(i, PermissionStatus::DENIED); |
+ break; |
+ } |
} |
return request_id; |
} |
-int AwPermissionManager::RequestPermissions( |
- const std::vector<PermissionType>& permissions, |
- content::RenderFrameHost* render_frame_host, |
- const GURL& requesting_origin, |
- bool user_gesture, |
- const base::Callback<void( |
- const std::vector<PermissionStatus>&)>& callback) { |
- NOTIMPLEMENTED() << "RequestPermissions has not been implemented in WebView"; |
- |
- std::vector<PermissionStatus> result(permissions.size()); |
- const GURL& embedding_origin = |
- content::WebContents::FromRenderFrameHost(render_frame_host) |
- ->GetLastCommittedURL().GetOrigin(); |
- |
- for (PermissionType type : permissions) { |
- result.push_back(GetPermissionStatus( |
- type, requesting_origin, embedding_origin)); |
- } |
- |
- callback.Run(result); |
- return kNoPendingOperation; |
-} |
- |
// static |
void AwPermissionManager::OnRequestResponse( |
const base::WeakPtr<AwPermissionManager>& manager, |
int request_id, |
- const base::Callback<void(PermissionStatus)>& callback, |
+ int request_index, |
bool allowed) { |
+ if (!manager.get()) |
+ return; |
+ |
PermissionStatus status = |
allowed ? PermissionStatus::GRANTED : PermissionStatus::DENIED; |
- if (manager.get()) { |
- PendingRequest* pending_request = |
- manager->pending_requests_.Lookup(request_id); |
- |
- for (PendingRequestsMap::Iterator<PendingRequest> it( |
- &manager->pending_requests_); |
- !it.IsAtEnd(); it.Advance()) { |
- if (pending_request->permission == it.GetCurrentValue()->permission && |
- it.GetCurrentKey() != request_id) { |
- it.GetCurrentValue()->callback.Run(status); |
- manager->pending_requests_.Remove(it.GetCurrentKey()); |
- } |
+ PendingRequest* pending_request = |
+ manager->pending_requests_.Lookup(request_id); |
+ PermissionType permission_type = |
+ pending_request->permissions()[request_index]; |
+ |
+ manager->result_cache_->SetResult( |
+ permission_type, |
+ pending_request->requesting_origin(), |
+ pending_request->embedding_origin(), |
+ status); |
+ |
+ std::vector<int> complete_request_ids; |
+ std::vector<std::pair<const RequestPermissionsCallback, |
+ std::vector<PermissionStatus>>> complete_request_pairs; |
+ for (PendingRequestsMap::Iterator<PendingRequest> it( |
+ &manager->pending_requests_); |
+ !it.IsAtEnd(); it.Advance()) { |
+ it.GetCurrentValue()->SetPermissionStatus(permission_type, status); |
+ if (it.GetCurrentValue()->IsComplete()) { |
+ complete_request_ids.push_back(it.GetCurrentKey()); |
+ complete_request_pairs.push_back( |
+ std::make_pair(it.GetCurrentValue()->callback(), |
+ it.GetCurrentValue()->results())); |
} |
- |
- manager->result_cache_->SetResult( |
- pending_request->permission, |
- pending_request->requesting_origin, |
- pending_request->embedding_origin, |
- status); |
- manager->pending_requests_.Remove(request_id); |
} |
- callback.Run(status); |
+ for (auto id : complete_request_ids) |
+ manager->pending_requests_.Remove(id); |
+ for (auto pair : complete_request_pairs) |
+ pair.first.Run(pair.second); |
} |
void AwPermissionManager::CancelPermissionRequest(int request_id) { |
@@ -319,63 +393,59 @@ void AwPermissionManager::CancelPermissionRequest(int request_id) { |
if (!pending_request) |
return; |
- content::RenderFrameHost* render_frame_host = |
- content::RenderFrameHost::FromID(pending_request->render_process_id, |
- pending_request->render_frame_id); |
- content::WebContents* web_contents = |
- content::WebContents::FromRenderFrameHost(render_frame_host); |
- DCHECK(web_contents); |
- |
// The caller is canceling (presumably) the most recent request. Assuming the |
// request did not complete, the user did not respond to the requset. |
// Thus, assume we do not know the result. |
- const GURL& embedding_origin = web_contents |
- ->GetLastCommittedURL().GetOrigin(); |
- result_cache_->ClearResult( |
- pending_request->permission, |
- pending_request->requesting_origin, |
- embedding_origin); |
+ const GURL& embedding_origin = pending_request->embedding_origin(); |
+ const GURL& requesting_origin = pending_request->requesting_origin(); |
+ for (auto permission : pending_request->permissions()) { |
+ result_cache_->ClearResult( |
+ permission, |
+ requesting_origin, |
+ embedding_origin); |
+ } |
AwBrowserPermissionRequestDelegate* delegate = |
AwBrowserPermissionRequestDelegate::FromID( |
- pending_request->render_process_id, |
- pending_request->render_frame_id); |
+ pending_request->render_process_id(), |
+ pending_request->render_frame_id()); |
if (!delegate) { |
pending_requests_.Remove(request_id); |
return; |
} |
- switch (pending_request->permission) { |
- case PermissionType::GEOLOCATION: |
- delegate->CancelGeolocationPermissionRequests( |
- pending_request->requesting_origin); |
- break; |
- case PermissionType::PROTECTED_MEDIA_IDENTIFIER: |
- delegate->CancelProtectedMediaIdentifierPermissionRequests( |
- pending_request->requesting_origin); |
- break; |
- case PermissionType::MIDI_SYSEX: |
- delegate->CancelMIDISysexPermissionRequests( |
- pending_request->requesting_origin); |
- break; |
- case PermissionType::NOTIFICATIONS: |
- case PermissionType::PUSH_MESSAGING: |
- case PermissionType::DURABLE_STORAGE: |
- case PermissionType::AUDIO_CAPTURE: |
- case PermissionType::VIDEO_CAPTURE: |
- case PermissionType::BACKGROUND_SYNC: |
- NOTIMPLEMENTED() << "CancelPermission not implemented for " |
- << static_cast<int>(pending_request->permission); |
- break; |
- case PermissionType::MIDI: |
- // There is nothing to cancel so this is simply ignored. |
- break; |
- case PermissionType::NUM: |
- NOTREACHED() << "PermissionType::NUM was not expected here."; |
- break; |
+ for (auto permission : pending_request->permissions()) { |
+ switch (permission) { |
+ case PermissionType::GEOLOCATION: |
+ delegate->CancelGeolocationPermissionRequests( |
+ requesting_origin); |
+ break; |
+ case PermissionType::PROTECTED_MEDIA_IDENTIFIER: |
+ delegate->CancelProtectedMediaIdentifierPermissionRequests( |
+ requesting_origin); |
+ break; |
+ case PermissionType::MIDI_SYSEX: |
+ delegate->CancelMIDISysexPermissionRequests( |
+ requesting_origin); |
+ break; |
+ case PermissionType::NOTIFICATIONS: |
+ case PermissionType::PUSH_MESSAGING: |
+ case PermissionType::DURABLE_STORAGE: |
+ case PermissionType::AUDIO_CAPTURE: |
+ case PermissionType::VIDEO_CAPTURE: |
+ case PermissionType::BACKGROUND_SYNC: |
+ NOTIMPLEMENTED() << "CancelPermission not implemented for " |
+ << static_cast<int>(permission); |
+ break; |
+ case PermissionType::MIDI: |
+ // There is nothing to cancel so this is simply ignored. |
+ break; |
+ case PermissionType::NUM: |
+ NOTREACHED() << "PermissionType::NUM was not expected here."; |
+ break; |
+ } |
+ pending_requests_.Remove(request_id); |
Tobias Sargeant
2016/07/12 12:43:23
Should this call be outside the loop?
Takashi Toyoshima
2016/07/13 06:00:10
Right. Thank you for catching this bug too.
|
} |
- |
- pending_requests_.Remove(request_id); |
} |
void AwPermissionManager::ResetPermission(PermissionType permission, |