 Chromium Code Reviews
 Chromium Code Reviews Issue 1194003004:
  Oilpan: enable appcache + move DocumentLoader to the heap.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/blink.git@master
    
  
    Issue 1194003004:
  Oilpan: enable appcache + move DocumentLoader to the heap.  (Closed) 
  Base URL: https://chromium.googlesource.com/chromium/blink.git@master| Index: Source/core/loader/DocumentLoader.cpp | 
| diff --git a/Source/core/loader/DocumentLoader.cpp b/Source/core/loader/DocumentLoader.cpp | 
| index 538b97b3a3e1acad8c52c4e5fd7acd7bffa47683..c2c125a342de5d4efc55428a3aa0f9c0065324a3 100644 | 
| --- a/Source/core/loader/DocumentLoader.cpp | 
| +++ b/Source/core/loader/DocumentLoader.cpp | 
| @@ -100,20 +100,31 @@ DocumentLoader::DocumentLoader(LocalFrame* frame, const ResourceRequest& req, co | 
| FrameLoader* DocumentLoader::frameLoader() const | 
| { | 
| if (!m_frame) | 
| - return 0; | 
| + return nullptr; | 
| return &m_frame->loader(); | 
| } | 
| ResourceLoader* DocumentLoader::mainResourceLoader() const | 
| { | 
| - return m_mainResource ? m_mainResource->loader() : 0; | 
| + return m_mainResource ? m_mainResource->loader() : nullptr; | 
| } | 
| DocumentLoader::~DocumentLoader() | 
| { | 
| ASSERT(!m_frame || !isLoading()); | 
| - clearMainResourceHandle(); | 
| + ASSERT(!m_mainResource); | 
| +#if !ENABLE(OILPAN) | 
| m_applicationCacheHost->dispose(); | 
| 
haraken
2015/06/22 01:57:58
I'm wondering if we could move this to detachFromF
 | 
| +#endif | 
| +} | 
| + | 
| +DEFINE_TRACE(DocumentLoader) | 
| +{ | 
| + visitor->trace(m_fetcher); | 
| + // TODO(sof): start tracing ResourcePtr<>s (and m_mainResource.) | 
| 
haraken
2015/06/22 01:57:58
I don't quite remember the current status of Resou
 
sof
2015/06/22 06:49:18
We definitely ought to when present on Oilpan heap
 | 
| + visitor->trace(m_writer); | 
| + visitor->trace(m_archive); | 
| + visitor->trace(m_applicationCacheHost); | 
| } | 
| unsigned long DocumentLoader::mainResourceIdentifier() const | 
| @@ -125,7 +136,7 @@ Document* DocumentLoader::document() const | 
| { | 
| if (m_frame && m_frame->loader().documentLoader() == this) | 
| return m_frame->document(); | 
| - return 0; | 
| + return nullptr; | 
| } | 
| const ResourceRequest& DocumentLoader::originalRequest() const | 
| @@ -206,7 +217,7 @@ void DocumentLoader::mainReceivedError(const ResourceError& error) | 
| void DocumentLoader::stopLoading() | 
| { | 
| RefPtrWillBeRawPtr<LocalFrame> protectFrame(m_frame); | 
| - RefPtr<DocumentLoader> protectLoader(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protectLoader(this); | 
| m_fetcher->stopFetching(); | 
| if (isLoading()) | 
| @@ -234,7 +245,7 @@ void DocumentLoader::notifyFinished(Resource* resource) | 
| ASSERT_UNUSED(resource, m_mainResource == resource); | 
| ASSERT(m_mainResource); | 
| - RefPtr<DocumentLoader> protect(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protect(this); | 
| if (!m_mainResource->errorOccurred() && !m_mainResource->wasCanceled()) { | 
| finishedLoading(m_mainResource->loadFinishTime()); | 
| @@ -248,7 +259,7 @@ void DocumentLoader::finishedLoading(double finishTime) | 
| { | 
| ASSERT(!mainResourceLoader() || !mainResourceLoader()->defersLoading() || InspectorInstrumentation::isDebuggerPaused(m_frame)); | 
| - RefPtr<DocumentLoader> protect(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protect(this); | 
| double responseEndTime = finishTime; | 
| if (!responseEndTime) | 
| @@ -436,7 +447,7 @@ void DocumentLoader::responseReceived(Resource* resource, const ResourceResponse | 
| { | 
| ASSERT_UNUSED(resource, m_mainResource == resource); | 
| ASSERT_UNUSED(handle, !handle); | 
| - RefPtr<DocumentLoader> protect(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protect(this); | 
| ASSERT(frame()); | 
| m_applicationCacheHost->didReceiveResponseForMainResource(response); | 
| @@ -542,7 +553,7 @@ void DocumentLoader::dataReceived(Resource* resource, const char* data, unsigned | 
| // Both unloading the old page and parsing the new page may execute JavaScript which destroys the datasource | 
| // by starting a new load, so retain temporarily. | 
| RefPtrWillBeRawPtr<LocalFrame> protectFrame(m_frame); | 
| - RefPtr<DocumentLoader> protectLoader(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protectLoader(this); | 
| m_applicationCacheHost->mainResourceDataReceived(data, length); | 
| m_timeOfLastDataReceived = monotonicallyIncreasingTime(); | 
| @@ -579,7 +590,7 @@ void DocumentLoader::detachFromFrame() | 
| { | 
| ASSERT(m_frame); | 
| RefPtrWillBeRawPtr<LocalFrame> protectFrame(m_frame); | 
| - RefPtr<DocumentLoader> protectLoader(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protectLoader(this); | 
| // It never makes sense to have a document loader that is detached from its | 
| // frame have any loads active, so go ahead and kill all the loads. | 
| @@ -588,7 +599,8 @@ void DocumentLoader::detachFromFrame() | 
| m_fetcher->clearContext(); | 
| m_applicationCacheHost->setApplicationCache(0); | 
| WeakIdentifierMap<DocumentLoader>::notifyObjectDestroyed(this); | 
| - m_frame = 0; | 
| + m_frame = nullptr; | 
| + clearMainResourceHandle(); | 
| } | 
| void DocumentLoader::clearMainResourceLoader() | 
| @@ -601,7 +613,7 @@ void DocumentLoader::clearMainResourceHandle() | 
| if (!m_mainResource) | 
| return; | 
| m_mainResource->removeClient(this); | 
| - m_mainResource = 0; | 
| + m_mainResource = nullptr; | 
| } | 
| bool DocumentLoader::maybeCreateArchive() | 
| @@ -691,7 +703,7 @@ bool DocumentLoader::maybeLoadEmpty() | 
| void DocumentLoader::startLoadingMainResource() | 
| { | 
| - RefPtr<DocumentLoader> protect(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protect(this); | 
| m_mainDocumentError = ResourceError(); | 
| timing().markNavigationStart(); | 
| ASSERT(!m_mainResource); | 
| @@ -741,7 +753,7 @@ void DocumentLoader::startLoadingMainResource() | 
| void DocumentLoader::cancelMainResourceLoad(const ResourceError& resourceError) | 
| { | 
| - RefPtr<DocumentLoader> protect(this); | 
| + RefPtrWillBeRawPtr<DocumentLoader> protect(this); | 
| ResourceError error = resourceError.isNull() ? ResourceError::cancelledError(m_request.url()) : resourceError; | 
| if (mainResourceLoader()) |