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

Side by Side Diff: extensions/renderer/dispatcher.cc

Issue 1318643002: Pass the v8::Context through the willDestroyServiceWorkerContextOnWorkerThread event. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: rebase against blink change Created 5 years, 3 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 unified diff | Download patch
« no previous file with comments | « extensions/renderer/dispatcher.h ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "extensions/renderer/dispatcher.h" 5 #include "extensions/renderer/dispatcher.h"
6 6
7 #include "base/bind.h" 7 #include "base/bind.h"
8 #include "base/callback.h" 8 #include "base/callback.h"
9 #include "base/command_line.h" 9 #include "base/command_line.h"
10 #include "base/containers/scoped_ptr_map.h" 10 #include "base/containers/scoped_ptr_map.h"
(...skipping 179 matching lines...) Expand 10 before | Expand all | Expand 10 after
190 void GetChrome(const v8::FunctionCallbackInfo<v8::Value>& args) { 190 void GetChrome(const v8::FunctionCallbackInfo<v8::Value>& args) {
191 args.GetReturnValue().Set(GetOrCreateChrome(context())); 191 args.GetReturnValue().Set(GetOrCreateChrome(context()));
192 } 192 }
193 }; 193 };
194 194
195 class ServiceWorkerScriptContextSet { 195 class ServiceWorkerScriptContextSet {
196 public: 196 public:
197 ServiceWorkerScriptContextSet() {} 197 ServiceWorkerScriptContextSet() {}
198 ~ServiceWorkerScriptContextSet() {} 198 ~ServiceWorkerScriptContextSet() {}
199 199
200 void Insert(const GURL& url, scoped_ptr<ScriptContext> context) { 200 void Insert(scoped_ptr<ScriptContext> context) {
201 base::AutoLock lock(lock_); 201 base::AutoLock lock(lock_);
202 CHECK(script_contexts_.find(url) == script_contexts_.end()); 202 CHECK(FindScriptContext(context->v8_context()) == contexts_.end());
203 script_contexts_.set(url, context.Pass()); 203 contexts_.push_back(context.Pass());
204 } 204 }
205 205
206 void Remove(const GURL& url) { 206 void Remove(v8::Local<v8::Context> v8_context) {
207 base::AutoLock lock(lock_); 207 base::AutoLock lock(lock_);
208 scoped_ptr<ScriptContext> context = script_contexts_.take_and_erase(url); 208 ScriptContextList::iterator context_it = FindScriptContext(v8_context);
209 CHECK(context); 209 // TODO(kalman): It would be good to CHECK(context_it != contexts_.end())
210 context->Invalidate(); 210 // here, but service workers can be started before the extension has been
211 // installed. See the length comment explaining why this happens, and
212 // how to solve it, in DidInitializeServiceWorkerContextOnWorkerThread.
213 // This does need to be fixed eventually, but for now, at least don't crash.
214 if (context_it == contexts_.end())
215 return;
216 (*context_it)->Invalidate();
217 contexts_.erase(context_it);
211 } 218 }
212 219
213 private: 220 private:
214 base::ScopedPtrMap<GURL, scoped_ptr<ScriptContext>> script_contexts_; 221 using ScriptContextList = ScopedVector<ScriptContext>;
222
223 // Returns an iterator to the ScriptContext associated with |v8_context|, or
224 // contexts_.end() if not found.
225 ScriptContextList::iterator FindScriptContext(
226 v8::Local<v8::Context> v8_context) {
227 for (auto it = contexts_.begin(); it != contexts_.end(); ++it) {
228 if ((*it)->v8_context() == v8_context)
229 return it;
230 }
231 return contexts_.end();
232 }
233
234 ScriptContextList contexts_;
215 235
216 mutable base::Lock lock_; 236 mutable base::Lock lock_;
217 237
218 DISALLOW_COPY_AND_ASSIGN(ServiceWorkerScriptContextSet); 238 DISALLOW_COPY_AND_ASSIGN(ServiceWorkerScriptContextSet);
219 }; 239 };
220 240
221 base::LazyInstance<ServiceWorkerScriptContextSet> 241 base::LazyInstance<ServiceWorkerScriptContextSet>
222 g_service_worker_script_context_set = LAZY_INSTANCE_INITIALIZER; 242 g_service_worker_script_context_set = LAZY_INSTANCE_INITIALIZER;
223 243
224 } // namespace 244 } // namespace
(...skipping 151 matching lines...) Expand 10 before | Expand all | Expand 10 after
376 396
377 // static 397 // static
378 void Dispatcher::DidInitializeServiceWorkerContextOnWorkerThread( 398 void Dispatcher::DidInitializeServiceWorkerContextOnWorkerThread(
379 v8::Local<v8::Context> v8_context, 399 v8::Local<v8::Context> v8_context,
380 const GURL& url) { 400 const GURL& url) {
381 const base::TimeTicks start_time = base::TimeTicks::Now(); 401 const base::TimeTicks start_time = base::TimeTicks::Now();
382 402
383 const Extension* extension = 403 const Extension* extension =
384 RendererExtensionRegistry::Get()->GetExtensionOrAppByURL(url); 404 RendererExtensionRegistry::Get()->GetExtensionOrAppByURL(url);
385 405
386 if (!extension) 406 if (!extension) {
407 // TODO(kalman): This is no good. Instead we need to either:
408 //
409 // - Hold onto the v8::Context and create the ScriptContext and install
410 // our bindings when this extension is loaded.
411 // - Deal with there being an extension ID (url.host()) but no
412 // extension associated with it, then document that getBackgroundClient
413 // may fail if the extension hasn't loaded yet.
414 //
415 // The former is safer, but is unfriendly to caching (e.g. session restore).
416 // It seems to contradict the service worker idiom.
417 //
418 // The latter is friendly to caching, but running extension code without an
419 // installed extension makes me nervous, and means that we won't be able to
420 // expose arbitrary (i.e. capability-checked) extension APIs to service
421 // workers. We will probably need to relax some assertions - we just need
422 // to find them.
423 //
424 // Perhaps this could be solved with our own event on the service worker
425 // saying that an extension is ready, and documenting that extension APIs
426 // won't work before that event has fired?
387 return; 427 return;
428 }
388 429
389 ScriptContext* context = new ScriptContext( 430 ScriptContext* context = new ScriptContext(
390 v8_context, nullptr, extension, Feature::SERVICE_WORKER_CONTEXT, 431 v8_context, nullptr, extension, Feature::SERVICE_WORKER_CONTEXT,
391 extension, Feature::SERVICE_WORKER_CONTEXT); 432 extension, Feature::SERVICE_WORKER_CONTEXT);
392 433
393 g_service_worker_script_context_set.Get().Insert(url, 434 g_service_worker_script_context_set.Get().Insert(make_scoped_ptr(context));
394 make_scoped_ptr(context));
395 435
396 v8::Isolate* isolate = context->isolate(); 436 v8::Isolate* isolate = context->isolate();
397 437
398 // Fetch the source code for service_worker_bindings.js. 438 // Fetch the source code for service_worker_bindings.js.
399 base::StringPiece script_resource = 439 base::StringPiece script_resource =
400 ResourceBundle::GetSharedInstance().GetRawDataResource( 440 ResourceBundle::GetSharedInstance().GetRawDataResource(
401 IDR_SERVICE_WORKER_BINDINGS_JS); 441 IDR_SERVICE_WORKER_BINDINGS_JS);
402 v8::Local<v8::String> script = v8::String::NewExternal( 442 v8::Local<v8::String> script = v8::String::NewExternal(
403 isolate, new StaticV8ExternalOneByteStringResource(script_resource)); 443 isolate, new StaticV8ExternalOneByteStringResource(script_resource));
404 444
(...skipping 26 matching lines...) Expand all
431 // TODO(kalman): Make |request_sender| use |context->AddInvalidationObserver|. 471 // TODO(kalman): Make |request_sender| use |context->AddInvalidationObserver|.
432 // In fact |request_sender_| should really be owned by ScriptContext. 472 // In fact |request_sender_| should really be owned by ScriptContext.
433 request_sender_->InvalidateSource(context); 473 request_sender_->InvalidateSource(context);
434 474
435 script_context_set_->Remove(context); 475 script_context_set_->Remove(context);
436 VLOG(1) << "Num tracked contexts: " << script_context_set_->size(); 476 VLOG(1) << "Num tracked contexts: " << script_context_set_->size();
437 } 477 }
438 478
439 // static 479 // static
440 void Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread( 480 void Dispatcher::WillDestroyServiceWorkerContextOnWorkerThread(
481 v8::Local<v8::Context> v8_context,
441 const GURL& url) { 482 const GURL& url) {
442 if (RendererExtensionRegistry::Get()->GetExtensionOrAppByURL(url)) 483 if (url.SchemeIs(kExtensionScheme))
443 g_service_worker_script_context_set.Get().Remove(url); 484 g_service_worker_script_context_set.Get().Remove(v8_context);
444 } 485 }
445 486
446 void Dispatcher::DidCreateDocumentElement(blink::WebLocalFrame* frame) { 487 void Dispatcher::DidCreateDocumentElement(blink::WebLocalFrame* frame) {
447 // Note: use GetEffectiveDocumentURL not just frame->document()->url() 488 // Note: use GetEffectiveDocumentURL not just frame->document()->url()
448 // so that this also injects the stylesheet on about:blank frames that 489 // so that this also injects the stylesheet on about:blank frames that
449 // are hosted in the extension process. 490 // are hosted in the extension process.
450 GURL effective_document_url = ScriptContext::GetEffectiveDocumentURL( 491 GURL effective_document_url = ScriptContext::GetEffectiveDocumentURL(
451 frame, frame->document().url(), true /* match_about_blank */); 492 frame, frame->document().url(), true /* match_about_blank */);
452 493
453 const Extension* extension = 494 const Extension* extension =
(...skipping 1115 matching lines...) Expand 10 before | Expand all | Expand 10 after
1569 void Dispatcher::AddChannelSpecificFeatures() { 1610 void Dispatcher::AddChannelSpecificFeatures() {
1570 // chrome-extension: resources should be allowed to register a Service Worker. 1611 // chrome-extension: resources should be allowed to register a Service Worker.
1571 if (FeatureProvider::GetBehaviorFeature(BehaviorFeature::kServiceWorker) 1612 if (FeatureProvider::GetBehaviorFeature(BehaviorFeature::kServiceWorker)
1572 ->IsAvailableToEnvironment() 1613 ->IsAvailableToEnvironment()
1573 .is_available()) 1614 .is_available())
1574 WebSecurityPolicy::registerURLSchemeAsAllowingServiceWorkers( 1615 WebSecurityPolicy::registerURLSchemeAsAllowingServiceWorkers(
1575 WebString::fromUTF8(kExtensionScheme)); 1616 WebString::fromUTF8(kExtensionScheme));
1576 } 1617 }
1577 1618
1578 } // namespace extensions 1619 } // namespace extensions
OLDNEW
« no previous file with comments | « extensions/renderer/dispatcher.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698