- Revision
- 112144
- Author
- [email protected]
- Date
- 2012-03-26 13:34:00 -0700 (Mon, 26 Mar 2012)
Log Message
Simplify setting loading state in DocumentLoader
https://bugs.webkit.org/show_bug.cgi?id=82099
Reviewed by Adam Barth.
The logic for deciding what to return for DocumentLoader::isLoading()
is crazy. It's indirectly based on the ResourceLoaders that have
registered themselves with the DocumentLoader, but we can make that
relationship more direct.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::checkLoadComplete): Renamed from updateLoading, since it's not actually
updating anything anymore. It now calls DOMWindow::finishedLoading() if loading is in fact done.
(WebCore::DocumentLoader::startLoadingMainResource): The only reason this had a return value was to call
updateLoading() if loading failed. Just call checkLoadComplete() directly (this allows it to
be private, whereas updateLoading() was public).
(WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):
* loader/DocumentLoader.h:
(WebCore::DocumentLoader::isLoading): Rather than holding a separate bool, return based on the presence
of non-multipart ResourceLoaders directly.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::isLoading): Depend on DocumentLoader::isLoading() for the activeDocumentLoader(),
rather than indirectly the other way around.
(WebCore::FrameLoader::checkLoadCompleteForThisFrame): Remove several assertions that should now be
absolutely identical to the remaining !pdl->isLoading().
(WebCore::FrameLoader::continueLoadAfterWillSubmitForm):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (112143 => 112144)
--- trunk/Source/WebCore/ChangeLog 2012-03-26 20:13:39 UTC (rev 112143)
+++ trunk/Source/WebCore/ChangeLog 2012-03-26 20:34:00 UTC (rev 112144)
@@ -1,3 +1,32 @@
+2012-03-26 Nate Chapin <[email protected]>
+
+ Simplify setting loading state in DocumentLoader
+ https://bugs.webkit.org/show_bug.cgi?id=82099
+
+ Reviewed by Adam Barth.
+
+ The logic for deciding what to return for DocumentLoader::isLoading()
+ is crazy. It's indirectly based on the ResourceLoaders that have
+ registered themselves with the DocumentLoader, but we can make that
+ relationship more direct.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::checkLoadComplete): Renamed from updateLoading, since it's not actually
+ updating anything anymore. It now calls DOMWindow::finishedLoading() if loading is in fact done.
+ (WebCore::DocumentLoader::startLoadingMainResource): The only reason this had a return value was to call
+ updateLoading() if loading failed. Just call checkLoadComplete() directly (this allows it to
+ be private, whereas updateLoading() was public).
+ (WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):
+ * loader/DocumentLoader.h:
+ (WebCore::DocumentLoader::isLoading): Rather than holding a separate bool, return based on the presence
+ of non-multipart ResourceLoaders directly.
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::isLoading): Depend on DocumentLoader::isLoading() for the activeDocumentLoader(),
+ rather than indirectly the other way around.
+ (WebCore::FrameLoader::checkLoadCompleteForThisFrame): Remove several assertions that should now be
+ absolutely identical to the remaining !pdl->isLoading().
+ (WebCore::FrameLoader::continueLoadAfterWillSubmitForm):
+
2012-03-26 James Robinson <[email protected]>
Scrollable plugins not registered properly in ScrollingCoordinator
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (112143 => 112144)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2012-03-26 20:13:39 UTC (rev 112143)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2012-03-26 20:34:00 UTC (rev 112144)
@@ -90,7 +90,6 @@
, m_request(req)
, m_committed(false)
, m_isStopping(false)
- , m_loading(false)
, m_gotFirstByte(false)
, m_primaryLoadComplete(false)
, m_isClientRedirect(false)
@@ -213,10 +212,10 @@
// but not loads initiated by child frames' data sources -- that's the WebFrame's job.
void DocumentLoader::stopLoading()
{
- // In some rare cases, calling FrameLoader::stopLoading could set m_loading to false.
+ // In some rare cases, calling FrameLoader::stopLoading could cause isLoading() to return false.
// (This can happen when there's a single XMLHttpRequest currently loading and stopLoading causes it
// to stop loading. Because of this, we need to save it so we don't return early.
- bool loading = m_loading;
+ bool loading = isLoading();
if (m_committed) {
// Attempt to stop the frame if the document loader is loading, or if it is done loading but
@@ -236,7 +235,7 @@
if (!loading) {
// If something above restarted loading we might run into mysterious crashes like
// https://bugs.webkit.org/show_bug.cgi?id=62764 and <rdar://problem/9328684>
- ASSERT(!m_loading);
+ ASSERT(!isLoading());
return;
}
@@ -371,20 +370,14 @@
#endif
}
-void DocumentLoader::updateLoading()
+void DocumentLoader::checkLoadComplete()
{
- if (!m_frame) {
- setLoading(false);
+ if (!m_frame || isLoading())
return;
- }
ASSERT(this == frameLoader()->activeDocumentLoader());
- bool wasLoading = m_loading;
- setLoading(frameLoader()->isLoading());
- if (wasLoading && !m_loading) {
- if (DOMWindow* window = m_frame->existingDOMWindow())
- window->finishedLoading();
- }
+ if (DOMWindow* window = m_frame->existingDOMWindow())
+ window->finishedLoading();
}
void DocumentLoader::setFrame(Frame* frame)
@@ -423,9 +416,6 @@
setPrimaryLoadComplete(false);
ASSERT(frameLoader());
clearErrors();
-
- setLoading(true);
-
frameLoader()->prepareForLoadStart();
}
@@ -439,7 +429,7 @@
}
if (this == frameLoader()->activeDocumentLoader())
- updateLoading();
+ checkLoadComplete();
}
}
@@ -770,13 +760,12 @@
void DocumentLoader::addSubresourceLoader(ResourceLoader* loader)
{
m_subresourceLoaders.add(loader);
- setLoading(true);
}
void DocumentLoader::removeSubresourceLoader(ResourceLoader* loader)
{
m_subresourceLoaders.remove(loader);
- updateLoading();
+ checkLoadComplete();
if (Frame* frame = m_frame)
frame->loader()->checkLoadComplete();
}
@@ -784,13 +773,12 @@
void DocumentLoader::addPlugInStreamLoader(ResourceLoader* loader)
{
m_plugInStreamLoaders.add(loader);
- setLoading(true);
}
void DocumentLoader::removePlugInStreamLoader(ResourceLoader* loader)
{
m_plugInStreamLoaders.remove(loader);
- updateLoading();
+ checkLoadComplete();
}
bool DocumentLoader::isLoadingMainResource() const
@@ -798,22 +786,12 @@
return !!m_mainResourceLoader;
}
-bool DocumentLoader::isLoadingSubresources() const
-{
- return !m_subresourceLoaders.isEmpty();
-}
-
-bool DocumentLoader::isLoadingPlugIns() const
-{
- return !m_plugInStreamLoaders.isEmpty();
-}
-
bool DocumentLoader::isLoadingMultipartContent() const
{
return m_mainResourceLoader && m_mainResourceLoader->isLoadingMultipartContent();
}
-bool DocumentLoader::startLoadingMainResource(unsigned long identifier)
+void DocumentLoader::startLoadingMainResource(unsigned long identifier)
{
ASSERT(!m_mainResourceLoader);
m_mainResourceLoader = MainResourceLoader::create(m_frame);
@@ -830,10 +808,9 @@
// should it be caught by other parts of WebKit or other parts of the app?
LOG_ERROR("could not create WebResourceHandle for URL %s -- should be caught by policy handler level", m_request.url().string().ascii().data());
m_mainResourceLoader = 0;
- return false;
+ ASSERT(!isLoading());
+ checkLoadComplete();
}
-
- return true;
}
void DocumentLoader::cancelMainResourceLoad(const ResourceError& error)
@@ -845,7 +822,7 @@
{
m_multipartSubresourceLoaders.add(loader);
m_subresourceLoaders.remove(loader);
- updateLoading();
+ checkLoadComplete();
if (Frame* frame = m_frame)
frame->loader()->checkLoadComplete();
}
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (112143 => 112144)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2012-03-26 20:13:39 UTC (rev 112143)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2012-03-26 20:34:00 UTC (rev 112144)
@@ -110,9 +110,7 @@
void stopLoading();
void setCommitted(bool committed) { m_committed = committed; }
bool isCommitted() const { return m_committed; }
- bool isLoading() const { return m_loading; }
- void setLoading(bool loading) { m_loading = loading; }
- void updateLoading();
+ bool isLoading() const { return isLoadingMainResource() || !m_subresourceLoaders.isEmpty() || !m_plugInStreamLoaders.isEmpty(); }
void receivedData(const char*, int);
void setupForReplaceByMIMEType(const String& newMIMEType);
void finishedLoading();
@@ -195,7 +193,7 @@
void setDefersLoading(bool);
- bool startLoadingMainResource(unsigned long identifier);
+ void startLoadingMainResource(unsigned long identifier);
void cancelMainResourceLoad(const ResourceError&);
// Support iconDatabase in synchronous mode.
@@ -207,8 +205,6 @@
void getIconDataForIconURL(const String&);
bool isLoadingMainResource() const;
- bool isLoadingSubresources() const;
- bool isLoadingPlugIns() const;
bool isLoadingMultipartContent() const;
void stopLoadingPlugIns();
@@ -262,6 +258,7 @@
void setMainDocumentError(const ResourceError&);
void commitLoad(const char*, int);
bool doesProgressiveLoad(const String& MIMEType) const;
+ void checkLoadComplete();
void deliverSubstituteResourcesAfterDelay();
void substituteResourceDeliveryTimerFired(Timer<DocumentLoader>*);
@@ -300,7 +297,6 @@
bool m_committed;
bool m_isStopping;
- bool m_loading;
bool m_gotFirstByte;
bool m_primaryLoadComplete;
bool m_isClientRedirect;
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (112143 => 112144)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2012-03-26 20:13:39 UTC (rev 112143)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2012-03-26 20:34:00 UTC (rev 112144)
@@ -1638,7 +1638,7 @@
DocumentLoader* docLoader = activeDocumentLoader();
if (!docLoader)
return false;
- return docLoader->isLoadingMainResource() || docLoader->isLoadingSubresources() || docLoader->isLoadingPlugIns();
+ return docLoader->isLoading();
}
bool FrameLoader::frameHasLoaded() const
@@ -2212,9 +2212,6 @@
m_delegateIsHandlingProvisionalLoadError = false;
ASSERT(!pdl->isLoading());
- ASSERT(!pdl->isLoadingMainResource());
- ASSERT(!pdl->isLoadingSubresources());
- ASSERT(!pdl->isLoadingPlugIns());
// If we're in the middle of loading multipart data, we need to restore the document loader.
if (isReplacing() && !m_documentLoader.get())
@@ -2316,9 +2313,7 @@
}
m_provisionalDocumentLoader->timing()->markNavigationStart(frame());
-
- if (!m_provisionalDocumentLoader->startLoadingMainResource(identifier))
- m_provisionalDocumentLoader->updateLoading();
+ m_provisionalDocumentLoader->startLoadingMainResource(identifier);
}
static KURL originatingURLFromBackForwardList(Page* page)
