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

Side by Side Diff: third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp

Issue 2377953002: Make hasPendingActivity return false after the associated browsing context is detached (Closed)
Patch Set: temp Created 4 years, 2 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 | « no previous file | 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 /* 1 /*
2 * Copyright (C) 2009 Google Inc. All rights reserved. 2 * Copyright (C) 2009 Google Inc. All rights reserved.
3 * 3 *
4 * Redistribution and use in source and binary forms, with or without 4 * Redistribution and use in source and binary forms, with or without
5 * modification, are permitted provided that the following conditions are 5 * modification, are permitted provided that the following conditions are
6 * met: 6 * met:
7 * 7 *
8 * * Redistributions of source code must retain the above copyright 8 * * Redistributions of source code must retain the above copyright
9 * notice, this list of conditions and the following disclaimer. 9 * notice, this list of conditions and the following disclaimer.
10 * * Redistributions in binary form must reproduce the above 10 * * Redistributions in binary form must reproduce the above
(...skipping 154 matching lines...) Expand 10 before | Expand all | Expand 10 after
165 if (value->IsIndependent()) 165 if (value->IsIndependent())
166 return; 166 return;
167 167
168 v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New( 168 v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(
169 m_isolate, v8::Persistent<v8::Object>::Cast(*value)); 169 m_isolate, v8::Persistent<v8::Object>::Cast(*value));
170 DCHECK(V8DOMWrapper::hasInternalFieldsSet(wrapper)); 170 DCHECK(V8DOMWrapper::hasInternalFieldsSet(wrapper));
171 171
172 const WrapperTypeInfo* type = toWrapperTypeInfo(wrapper); 172 const WrapperTypeInfo* type = toWrapperTypeInfo(wrapper);
173 if (type->isActiveScriptWrappable() && 173 if (type->isActiveScriptWrappable() &&
174 toScriptWrappable(wrapper)->hasPendingActivity()) { 174 toScriptWrappable(wrapper)->hasPendingActivity()) {
175 m_isolate->SetObjectGroupId(*value, liveRootId()); 175 // Enable hasPendingActivity only when the associated
176 ++m_domObjectsWithPendingActivity; 176 // ExecutionContext is not yet detached. This is a work-around
177 // to avoid memory leaks caused by hasPendingActivity that keeps
178 // returning true forever. This will be okay in practice because
179 // the spec requires to stop almost all DOM activities when the
180 // associated browsing context is detached. However, the real
181 // problem is that some hasPendingActivity's are wrongly implemented
182 // and never return false.
183 // TODO(haraken): Implement correct lifetime using traceWrapper.
184 ExecutionContext* context =
185 toExecutionContext(wrapper->CreationContext());
186 if (context && !context->activeDOMObjectsAreStopped()) {
187 m_isolate->SetObjectGroupId(*value, liveRootId());
188 ++m_domObjectsWithPendingActivity;
189 }
177 } 190 }
178 191
179 if (classId == WrapperTypeInfo::NodeClassId) { 192 if (classId == WrapperTypeInfo::NodeClassId) {
180 DCHECK(V8Node::hasInstance(wrapper, m_isolate)); 193 DCHECK(V8Node::hasInstance(wrapper, m_isolate));
181 Node* node = V8Node::toImpl(wrapper); 194 Node* node = V8Node::toImpl(wrapper);
182 if (node->hasEventListeners()) 195 if (node->hasEventListeners())
183 addReferencesForNodeWithEventListeners( 196 addReferencesForNodeWithEventListeners(
184 m_isolate, node, v8::Persistent<v8::Object>::Cast(*value)); 197 m_isolate, node, v8::Persistent<v8::Object>::Cast(*value));
185 Node* root = V8GCController::opaqueRootForGC(m_isolate, node); 198 Node* root = V8GCController::opaqueRootForGC(m_isolate, node);
186 m_isolate->SetObjectGroupId( 199 m_isolate->SetObjectGroupId(
(...skipping 295 matching lines...) Expand 10 before | Expand all | Expand 10 after
482 495
483 if (classId != WrapperTypeInfo::NodeClassId && 496 if (classId != WrapperTypeInfo::NodeClassId &&
484 classId != WrapperTypeInfo::ObjectClassId) 497 classId != WrapperTypeInfo::ObjectClassId)
485 return; 498 return;
486 499
487 v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New( 500 v8::Local<v8::Object> wrapper = v8::Local<v8::Object>::New(
488 m_isolate, v8::Persistent<v8::Object>::Cast(*value)); 501 m_isolate, v8::Persistent<v8::Object>::Cast(*value));
489 ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper)); 502 ASSERT(V8DOMWrapper::hasInternalFieldsSet(wrapper));
490 // The ExecutionContext check is heavy, so it should be done at the last. 503 // The ExecutionContext check is heavy, so it should be done at the last.
491 if (toWrapperTypeInfo(wrapper)->isActiveScriptWrappable() && 504 if (toWrapperTypeInfo(wrapper)->isActiveScriptWrappable() &&
492 toScriptWrappable(wrapper)->hasPendingActivity() 505 toScriptWrappable(wrapper)->hasPendingActivity()) {
493 // TODO(haraken): Currently we don't have a way to get a creation 506 // See the comment in MajorGCWrapperVisitor::VisitPersistentHandle.
494 // context from a wrapper. We should implement the way and enable 507 ExecutionContext* context =
495 // the following condition. 508 toExecutionContext(wrapper->CreationContext());
496 // 509 if (context == m_executionContext && context &&
497 // This condition affects only compositor workers, where one isolate 510 !context->activeDOMObjectsAreStopped())
498 // is shared by multiple workers. If we don't have the condition, 511 m_pendingActivityFound = true;
499 // a worker object for a compositor worker doesn't get collected 512 }
500 // until all compositor workers in the same isolate lose pending
501 // activities. In other words, not having the condition delays
502 // destruction of a worker object of a compositor worker.
503 //
504 /* && toExecutionContext(wrapper->creationContext()) == m_executionConte xt */
505 )
506 m_pendingActivityFound = true;
507 } 513 }
508 514
509 bool pendingActivityFound() const { return m_pendingActivityFound; } 515 bool pendingActivityFound() const { return m_pendingActivityFound; }
510 516
511 private: 517 private:
512 v8::Isolate* m_isolate; 518 v8::Isolate* m_isolate;
513 Persistent<ExecutionContext> m_executionContext; 519 Persistent<ExecutionContext> m_executionContext;
514 bool m_pendingActivityFound; 520 bool m_pendingActivityFound;
515 }; 521 };
516 522
(...skipping 10 matching lines...) Expand all
527 double startTime = WTF::currentTimeMS(); 533 double startTime = WTF::currentTimeMS();
528 v8::HandleScope scope(isolate); 534 v8::HandleScope scope(isolate);
529 PendingActivityVisitor visitor(isolate, executionContext); 535 PendingActivityVisitor visitor(isolate, executionContext);
530 toIsolate(executionContext)->VisitHandlesWithClassIds(&visitor); 536 toIsolate(executionContext)->VisitHandlesWithClassIds(&visitor);
531 scanPendingActivityHistogram.count( 537 scanPendingActivityHistogram.count(
532 static_cast<int>(WTF::currentTimeMS() - startTime)); 538 static_cast<int>(WTF::currentTimeMS() - startTime));
533 return visitor.pendingActivityFound(); 539 return visitor.pendingActivityFound();
534 } 540 }
535 541
536 } // namespace blink 542 } // namespace blink
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698