Skip to content

Commit

Permalink
Revert of Revert of Make start/stop loading notifications per-frame (h…
Browse files Browse the repository at this point in the history 
…ttps://codereview.chromium.org/215533004/)

Reason for revert:
r170192 was not a culprit.


Original issue's description:
> Revert of Make start/stop loading notifications per-frame (https://codereview.chromium.org/183793002/)
> 
> Reason for revert:
> Possibly causing content_browsertests EndToEnd to fail.
> 
> See: http://build.chromium.org/p/chromium.webkit/builders/Linux%20Tests%20%28dbg%29/builds/2095/steps/content_browsertests/logs/stdio
> 
> Output:
> [20917:20917:0327/122353:3268961829:INFO:CONSOLE(0)] "Uncaught TypeError: Cannot convert undefined or null to object", source: http://127.0.0.1:34379/files/web_ui_mojo.html (0)
> [20917:20917:0327/122353:3269022437:INFO:CONSOLE(28)] "Uncaught TypeError: undefined is not a function", source: http://127.0.0.1:34379/files/web_ui_mojo.js (28)
> BrowserTestBase signal handler received SIGTERM. Backtrace:
> #0 0x000000a593be base::debug::StackTrace::StackTrace()
> #1 0x000002bfc17b content::(anonymous namespace)::DumpStackTraceSignalHandler()
> #2 0x7f9c3e4ab4a0 <unknown>
> #3 0x7f9c3e55da43 __poll
> #4 0x7f9c42aabff6 <unknown>
> #5 0x7f9c42aac124 g_main_context_iteration
> #6 0x000000aaaf5f base::MessagePumpGlib::RunWithDispatcher()
> #7 0x000000a8e802 base::RunLoop::Run()
> #8 0x00000058256e content::(anonymous namespace)::WebUIMojoTest_EndToEnd_Test::RunTestOnMainThread()
> 
> Original issue's description:
> > Make start/stop loading notifications per-frame
> > 
> > Also, make ProgressTracker a frame-level concept rather than a page-level concept and merge it with FrameLoader's FrameProgressTracker helper. Send per-frame progress change notifications accordingly.
> > 
> > This depends on https://codereview.chromium.org/180113003/ landing in chromium.
> > 
> > BUG=347643
> > 
> > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170192
> 
> TBR=abarth@chromium.org,japhet@chromium.org
> NOTREECHECKS=true
> NOTRY=true
> BUG=347643
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=170218

TBR=abarth@chromium.org,japhet@chromium.org,enne@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=347643

Review URL: https://codereview.chromium.org/216083002

git-svn-id: svn://svn.chromium.org/blink/trunk@170248 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
tkent@chromium.org committed Mar 28, 2014
1 parent 6b62d69 commit 28a1ff6
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 170 deletions.
4 changes: 2 additions & 2 deletions Source/core/accessibility/AXRenderObject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1493,8 +1493,8 @@ double AXRenderObject::estimatedLoadingProgress() const
if (isLoaded())
return 1.0;

if (Page* page = m_renderer->document().page())
return page->progress().estimatedProgress();
if (LocalFrame* frame = m_renderer->document().frame())
return frame->loader().progress().estimatedProgress();
return 0;
}

Expand Down
6 changes: 3 additions & 3 deletions Source/core/loader/EmptyClients.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ class EmptyFrameLoaderClient FINAL : public FrameLoaderClient {
virtual void dispatchWillSendSubmitEvent(HTMLFormElement*) OVERRIDE;
virtual void dispatchWillSubmitForm(HTMLFormElement*) OVERRIDE;

virtual void postProgressStartedNotification(LoadStartType) OVERRIDE { }
virtual void postProgressEstimateChangedNotification() OVERRIDE { }
virtual void postProgressFinishedNotification() OVERRIDE { }
virtual void didStartLoading(LoadStartType) OVERRIDE { }
virtual void progressEstimateChanged(double) OVERRIDE { }
virtual void didStopLoading() OVERRIDE { }

virtual void loadURLExternally(const ResourceRequest&, NavigationPolicy, const String& = String()) OVERRIDE { }

Expand Down
15 changes: 5 additions & 10 deletions Source/core/loader/FrameFetchContext.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,39 +159,34 @@ void FrameFetchContext::dispatchDidLoadResourceFromMemoryCache(const ResourceReq

void FrameFetchContext::dispatchDidReceiveResponse(DocumentLoader* loader, unsigned long identifier, const ResourceResponse& r, ResourceLoader* resourceLoader)
{
if (Page* page = m_frame->page())
page->progress().incrementProgress(identifier, r);
m_frame->loader().progress().incrementProgress(identifier, r);
m_frame->loader().client()->dispatchDidReceiveResponse(loader, identifier, r);
InspectorInstrumentation::didReceiveResourceResponse(m_frame, identifier, ensureLoader(loader), r, resourceLoader);
}

void FrameFetchContext::dispatchDidReceiveData(DocumentLoader*, unsigned long identifier, const char* data, int dataLength, int encodedDataLength)
{
if (Page* page = m_frame->page())
page->progress().incrementProgress(identifier, data, dataLength);
m_frame->loader().progress().incrementProgress(identifier, data, dataLength);
InspectorInstrumentation::didReceiveData(m_frame, identifier, data, dataLength, encodedDataLength);
}

void FrameFetchContext::dispatchDidDownloadData(DocumentLoader*, unsigned long identifier, int dataLength, int encodedDataLength)
{
if (Page* page = m_frame->page())
page->progress().incrementProgress(identifier, 0, dataLength);
m_frame->loader().progress().incrementProgress(identifier, 0, dataLength);
InspectorInstrumentation::didReceiveData(m_frame, identifier, 0, dataLength, encodedDataLength);
}

void FrameFetchContext::dispatchDidFinishLoading(DocumentLoader* loader, unsigned long identifier, double finishTime, int64_t encodedDataLength)
{
if (Page* page = m_frame->page())
page->progress().completeProgress(identifier);
m_frame->loader().progress().completeProgress(identifier);
m_frame->loader().client()->dispatchDidFinishLoading(loader, identifier);

InspectorInstrumentation::didFinishLoading(m_frame, identifier, ensureLoader(loader), finishTime, encodedDataLength);
}

void FrameFetchContext::dispatchDidFail(DocumentLoader* loader, unsigned long identifier, const ResourceError& error)
{
if (Page* page = m_frame->page())
page->progress().completeProgress(identifier);
m_frame->loader().progress().completeProgress(identifier);
InspectorInstrumentation::didFailLoading(m_frame, identifier, error);
}

Expand Down
54 changes: 3 additions & 51 deletions Source/core/loader/FrameLoader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,48 +105,11 @@ static bool needsHistoryItemRestore(FrameLoadType type)
return type == FrameLoadTypeBackForward || type == FrameLoadTypeReload || type == FrameLoadTypeReloadFromOrigin;
}

class FrameLoader::FrameProgressTracker {
public:
static PassOwnPtr<FrameProgressTracker> create(LocalFrame* frame) { return adoptPtr(new FrameProgressTracker(frame)); }
~FrameProgressTracker()
{
ASSERT(!m_inProgress || m_frame->page());
if (m_inProgress)
m_frame->page()->progress().progressCompleted(m_frame);
}

void progressStarted()
{
ASSERT(m_frame->page());
if (!m_inProgress)
m_frame->page()->progress().progressStarted(m_frame);
m_inProgress = true;
}

void progressCompleted()
{
ASSERT(m_inProgress);
ASSERT(m_frame->page());
m_inProgress = false;
m_frame->page()->progress().progressCompleted(m_frame);
}

private:
FrameProgressTracker(LocalFrame* frame)
: m_frame(frame)
, m_inProgress(false)
{
}

LocalFrame* m_frame;
bool m_inProgress;
};

FrameLoader::FrameLoader(LocalFrame* frame, FrameLoaderClient* client)
: m_frame(frame)
, m_client(client)
, m_mixedContentChecker(frame)
, m_progressTracker(FrameProgressTracker::create(m_frame))
, m_progressTracker(ProgressTracker::create(frame))
, m_state(FrameStateProvisional)
, m_loadType(FrameLoadTypeStandard)
, m_fetchContext(FrameFetchContext::create(frame))
Expand Down Expand Up @@ -573,14 +536,14 @@ void FrameLoader::updateForSameDocumentNavigation(const KURL& newURL, SameDocume
// don't fire them for fragment redirection that happens in window.onload handler.
// See https://bugs.webkit.org/show_bug.cgi?id=31838
if (m_frame->document()->loadEventFinished())
m_client->postProgressStartedNotification(NavigationWithinSameDocument);
m_client->didStartLoading(NavigationWithinSameDocument);

HistoryCommitType historyCommitType = updateBackForwardList == UpdateBackForwardList && m_currentItem ? StandardCommit : HistoryInertCommit;
setHistoryItemStateForCommit(historyCommitType, sameDocumentNavigationSource == SameDocumentNavigationHistoryApi, data);
m_client->dispatchDidNavigateWithinPage(m_currentItem.get(), historyCommitType);
m_client->dispatchDidReceiveTitle(m_frame->document()->title());
if (m_frame->document()->loadEventFinished())
m_client->postProgressFinishedNotification();
m_client->didStopLoading();
}

void FrameLoader::loadInSameDocument(const KURL& url, PassRefPtr<SerializedScriptValue> stateObject, UpdateBackForwardListPolicy updateBackForwardList, ClientRedirectPolicy clientRedirect)
Expand Down Expand Up @@ -1087,17 +1050,6 @@ void FrameLoader::checkLoadComplete(DocumentLoader* documentLoader)
checkLoadComplete();
}

int FrameLoader::numPendingOrLoadingRequests(bool recurse) const
{
if (!recurse)
return m_frame->document()->fetcher()->requestCount();

int count = 0;
for (LocalFrame* frame = m_frame; frame; frame = frame->tree().traverseNext(m_frame))
count += frame->document()->fetcher()->requestCount();
return count;
}

String FrameLoader::userAgent(const KURL& url) const
{
String userAgent = m_client->userAgent(url);
Expand Down
7 changes: 3 additions & 4 deletions Source/core/loader/FrameLoader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class FrameLoaderClient;
class IconController;
class NavigationAction;
class Page;
class ProgressTracker;
class ResourceError;
class ResourceResponse;
class SecurityOrigin;
Expand All @@ -82,6 +83,7 @@ class FrameLoader {
LocalFrame* frame() const { return m_frame; }

MixedContentChecker* mixedContentChecker() const { return &m_mixedContentChecker; }
ProgressTracker& progress() const { return *m_progressTracker; }

// These functions start a load. All eventually call into loadWithNavigationAction() or loadInSameDocument().
void load(const FrameLoadRequest&); // The entry point for non-reload, non-history loads.
Expand Down Expand Up @@ -111,8 +113,6 @@ class FrameLoader {

bool isLoading() const;

int numPendingOrLoadingRequests(bool recurse) const;

DocumentLoader* documentLoader() const { return m_documentLoader.get(); }
DocumentLoader* policyDocumentLoader() const { return m_policyDocumentLoader.get(); }
DocumentLoader* provisionalDocumentLoader() const { return m_provisionalDocumentLoader.get(); }
Expand Down Expand Up @@ -238,8 +238,7 @@ class FrameLoader {
mutable FrameLoaderStateMachine m_stateMachine;
mutable MixedContentChecker m_mixedContentChecker;

class FrameProgressTracker;
OwnPtr<FrameProgressTracker> m_progressTracker;
OwnPtr<ProgressTracker> m_progressTracker;

FrameState m_state;
FrameLoadType m_loadType;
Expand Down
7 changes: 3 additions & 4 deletions Source/core/loader/FrameLoaderClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,9 @@ namespace WebCore {
virtual void dispatchWillSendSubmitEvent(HTMLFormElement*) = 0;
virtual void dispatchWillSubmitForm(HTMLFormElement*) = 0;

// Maybe these should go into a ProgressTrackerClient some day
virtual void postProgressStartedNotification(LoadStartType) = 0;
virtual void postProgressEstimateChangedNotification() = 0;
virtual void postProgressFinishedNotification() = 0;
virtual void didStartLoading(LoadStartType) = 0;
virtual void progressEstimateChanged(double progressEstimate) = 0;
virtual void didStopLoading() = 0;

virtual void loadURLExternally(const ResourceRequest&, NavigationPolicy, const String& suggestedName = String()) = 0;

Expand Down
75 changes: 25 additions & 50 deletions Source/core/loader/ProgressTracker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "config.h"
#include "core/loader/ProgressTracker.h"

#include "core/fetch/ResourceFetcher.h"
#include "core/frame/FrameView.h"
#include "core/frame/LocalFrame.h"
#include "core/inspector/InspectorInstrumentation.h"
Expand Down Expand Up @@ -61,26 +62,29 @@ struct ProgressItem {
long long estimatedLength;
};

ProgressTracker::ProgressTracker()
: m_totalPageAndResourceBytesToLoad(0)
ProgressTracker::ProgressTracker(LocalFrame* frame)
: m_frame(frame)
, m_inProgress(false)
, m_totalPageAndResourceBytesToLoad(0)
, m_totalBytesReceived(0)
, m_lastNotifiedProgressValue(0)
, m_lastNotifiedProgressTime(0)
, m_progressNotificationInterval(0.02)
, m_progressNotificationTimeInterval(0.1)
, m_finalProgressChangedSent(false)
, m_progressValue(0)
, m_numProgressTrackedFrames(0)
{
}

ProgressTracker::~ProgressTracker()
{
if (m_inProgress)
progressCompleted();
}

PassOwnPtr<ProgressTracker> ProgressTracker::create()
PassOwnPtr<ProgressTracker> ProgressTracker::create(LocalFrame* frame)
{
return adoptPtr(new ProgressTracker);
return adoptPtr(new ProgressTracker(frame));
}

double ProgressTracker::estimatedProgress() const
Expand All @@ -98,59 +102,35 @@ void ProgressTracker::reset()
m_lastNotifiedProgressValue = 0;
m_lastNotifiedProgressTime = 0;
m_finalProgressChangedSent = false;
m_numProgressTrackedFrames = 0;
m_originatingProgressFrame = nullptr;
}

void ProgressTracker::progressStarted(LocalFrame* frame)
void ProgressTracker::progressStarted()
{
WTF_LOG(Progress, "Progress started (%p) - frame %p(\"%s\"), value %f, tracked frames %d, originating frame %p", this, frame, frame->tree().uniqueName().utf8().data(), m_progressValue, m_numProgressTrackedFrames, m_originatingProgressFrame.get());

if (m_numProgressTrackedFrames == 0 || m_originatingProgressFrame == frame) {
if (!m_inProgress) {
reset();
m_progressValue = initialProgressValue;
m_originatingProgressFrame = frame;

m_originatingProgressFrame->loader().client()->postProgressStartedNotification(NavigationToDifferentDocument);
m_frame->loader().client()->didStartLoading(NavigationToDifferentDocument);
}
m_numProgressTrackedFrames++;
InspectorInstrumentation::frameStartedLoading(frame);
}

void ProgressTracker::progressCompleted(LocalFrame* frame)
{
WTF_LOG(Progress, "Progress completed (%p) - frame %p(\"%s\"), value %f, tracked frames %d, originating frame %p", this, frame, frame->tree().uniqueName().utf8().data(), m_progressValue, m_numProgressTrackedFrames, m_originatingProgressFrame.get());

if (m_numProgressTrackedFrames <= 0)
return;
m_numProgressTrackedFrames--;
if (!m_numProgressTrackedFrames || m_originatingProgressFrame == frame)
finalProgressComplete();
m_inProgress = true;
InspectorInstrumentation::frameStartedLoading(m_frame);
}

void ProgressTracker::finalProgressComplete()
void ProgressTracker::progressCompleted()
{
WTF_LOG(Progress, "Final progress complete (%p)", this);

RefPtr<LocalFrame> frame = m_originatingProgressFrame.release();

// Before resetting progress value be sure to send client a least one notification
// with final progress value.
ASSERT(m_inProgress);
m_inProgress = false;
if (!m_finalProgressChangedSent) {
m_progressValue = 1;
frame->loader().client()->postProgressEstimateChangedNotification();
m_frame->loader().client()->progressEstimateChanged(m_progressValue);
}

reset();
frame->loader().client()->postProgressFinishedNotification();
InspectorInstrumentation::frameStoppedLoading(frame.get());
m_frame->loader().client()->didStopLoading();
InspectorInstrumentation::frameStoppedLoading(m_frame);
}

void ProgressTracker::incrementProgress(unsigned long identifier, const ResourceResponse& response)
{
WTF_LOG(Progress, "Progress incremented (%p) - value %f, tracked frames %d, originating frame %p", this, m_progressValue, m_numProgressTrackedFrames, m_originatingProgressFrame.get());

if (m_numProgressTrackedFrames <= 0)
if (!m_inProgress)
return;

long long estimatedLength = response.expectedContentLength();
Expand All @@ -174,8 +154,6 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
if (!item)
return;

RefPtr<LocalFrame> frame = m_originatingProgressFrame;

unsigned bytesReceived = length;
double increment, percentOfRemainingBytes;
long long remainingBytes, estimatedBytesForPendingRequests;
Expand All @@ -186,7 +164,7 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
item->estimatedLength = item->bytesReceived * 2;
}

int numPendingOrLoadingRequests = frame->loader().numPendingOrLoadingRequests(true);
int numPendingOrLoadingRequests = m_frame->document()->fetcher()->requestCount();
estimatedBytesForPendingRequests = progressItemDefaultEstimatedLength * numPendingOrLoadingRequests;
remainingBytes = ((m_totalPageAndResourceBytesToLoad + estimatedBytesForPendingRequests) - m_totalBytesReceived);
if (remainingBytes > 0) // Prevent divide by 0.
Expand All @@ -195,7 +173,7 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
percentOfRemainingBytes = 1.0;

// For documents that use WebCore's layout system, treat first layout as the half-way point.
bool useClampedMaxProgress = !frame->view()->didFirstLayout();
bool useClampedMaxProgress = !m_frame->view()->didFirstLayout();
double maxProgressValue = useClampedMaxProgress ? 0.5 : finalProgressValue;
increment = (maxProgressValue - m_progressValue) * percentOfRemainingBytes;
m_progressValue += increment;
Expand All @@ -207,16 +185,13 @@ void ProgressTracker::incrementProgress(unsigned long identifier, const char*, i
double now = currentTime();
double notifiedProgressTimeDelta = now - m_lastNotifiedProgressTime;

WTF_LOG(Progress, "Progress incremented (%p) - value %f, tracked frames %d", this, m_progressValue, m_numProgressTrackedFrames);
double notificationProgressDelta = m_progressValue - m_lastNotifiedProgressValue;
if ((notificationProgressDelta >= m_progressNotificationInterval ||
notifiedProgressTimeDelta >= m_progressNotificationTimeInterval) &&
m_numProgressTrackedFrames > 0) {
if (notificationProgressDelta >= m_progressNotificationInterval || notifiedProgressTimeDelta >= m_progressNotificationTimeInterval) {
if (!m_finalProgressChangedSent) {
if (m_progressValue == 1)
m_finalProgressChangedSent = true;

frame->loader().client()->postProgressEstimateChangedNotification();
m_frame->loader().client()->progressEstimateChanged(m_progressValue);

m_lastNotifiedProgressValue = m_progressValue;
m_lastNotifiedProgressTime = now;
Expand Down
Loading

0 comments on commit 28a1ff6

Please sign in to comment.