Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(40)

Unified Diff: extensions/browser/script_executor.cc

Issue 1216453002: [Extensions] Handle some funny cases in script injection (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Ben's Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/browser/script_executor.cc
diff --git a/extensions/browser/script_executor.cc b/extensions/browser/script_executor.cc
index 2b6164613d806145707d3b541152bd423a9871a7..eeff9e2385fe631ebf259bd3db191f5a75805c30 100644
--- a/extensions/browser/script_executor.cc
+++ b/extensions/browser/script_executor.cc
@@ -42,7 +42,6 @@ class Handler : public content::WebContentsObserver {
script_observers_(AsWeakPtr(script_observers)),
host_id_(params.host_id),
request_id_(params.request_id),
- num_pending_(0),
callback_(callback) {
if (scope == ScriptExecutor::ALL_FRAMES) {
web_contents->ForEachFrame(base::Bind(&Handler::SendExecuteCode,
@@ -57,9 +56,7 @@ class Handler : public content::WebContentsObserver {
~Handler() override {}
// content::WebContentsObserver:
- void WebContentsDestroyed() override {
- Finish(kRendererDestroyed, GURL(), base::ListValue());
- }
+ void WebContentsDestroyed() override { Finish(); }
bool OnMessageReceived(const IPC::Message& message,
content::RenderFrameHost* render_frame_host) override {
@@ -82,11 +79,19 @@ class Handler : public content::WebContentsObserver {
return true;
}
+ void RenderFrameDeleted(
+ content::RenderFrameHost* render_frame_host) override {
+ if (pending_render_frames_.erase(render_frame_host) == 1 &&
+ pending_render_frames_.empty()) {
+ Finish();
+ }
+ }
+
// Sends an ExecuteCode message to the given frame host, and increments
// the number of pending messages.
void SendExecuteCode(const ExtensionMsg_ExecuteCode_Params& params,
content::RenderFrameHost* frame) {
- ++num_pending_;
+ pending_render_frames_.insert(frame);
frame->Send(new ExtensionMsg_ExecuteCode(frame->GetRoutingID(), params));
}
@@ -97,7 +102,9 @@ class Handler : public content::WebContentsObserver {
const GURL& on_url,
const base::ListValue& result_list) {
DCHECK_EQ(request_id_, request_id);
- DCHECK_GT(num_pending_, 0);
+ DCHECK(!pending_render_frames_.empty());
+ bool erased = pending_render_frames_.erase(render_frame_host) == 1;
+ DCHECK(erased);
bool is_main_frame = web_contents()->GetMainFrame() == render_frame_host;
// Set the result, if there is one.
@@ -112,31 +119,33 @@ class Handler : public content::WebContentsObserver {
}
if (is_main_frame) { // Only use the main frame's error and url.
- error_ = error;
- url_ = on_url;
+ main_frame_error_ = error;
+ main_frame_url_ = on_url;
}
// Wait until the final request finishes before reporting back.
- if (--num_pending_ > 0)
- return;
+ if (pending_render_frames_.empty())
+ Finish();
+ }
+
+ void Finish() {
+ if (main_frame_url_.is_empty()) {
+ // We never finished the main frame injection.
+ main_frame_error_ = kRendererDestroyed;
+ results_.Clear();
+ }
- if (script_observers_.get() && error.empty() &&
+ if (script_observers_.get() && main_frame_error_.empty() &&
host_id_.type() == HostID::EXTENSIONS) {
ScriptExecutionObserver::ExecutingScriptsMap id_map;
id_map[host_id_.id()] = std::set<std::string>();
- FOR_EACH_OBSERVER(ScriptExecutionObserver,
- *script_observers_,
- OnScriptsExecuted(web_contents(), id_map, on_url));
+ FOR_EACH_OBSERVER(ScriptExecutionObserver, *script_observers_,
+ OnScriptsExecuted(web_contents(), id_map,
+ main_frame_url_));
}
- Finish(error_, url_, results_);
- }
-
- void Finish(const std::string& error,
- const GURL& url,
- const base::ListValue& result) {
if (!callback_.is_null())
- callback_.Run(error, url, result);
+ callback_.Run(main_frame_error_, main_frame_url_, results_);
delete this;
}
@@ -148,17 +157,17 @@ class Handler : public content::WebContentsObserver {
// The request id of the injection.
int request_id_;
- // The number of still-running injections.
- int num_pending_;
+ // The hosts of the still-running injections.
+ std::set<content::RenderFrameHost*> pending_render_frames_;
// The results of the injection.
base::ListValue results_;
// The error from injecting into the main frame.
- std::string error_;
+ std::string main_frame_error_;
// The url of the main frame.
- GURL url_;
+ GURL main_frame_url_;
// The callback to run after all injections complete.
ScriptExecutor::ExecuteScriptCallback callback_;

Powered by Google App Engine
This is Rietveld 408576698