Bug 104168 – Web Inspector: Add events for tracking when a page is about to start loading or is loading

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Ken Kania]

We'd like to be able to be notified when a page is about to start loading or is loading. I propose adding 4 hidden navigation scheduling events to the remote debugging protocol:

1) Page.loadingStarted
This event will be sent in WebCore::ProgressTracker::progressStarted. No parameters. For now it will only be sent for the main frame.

2) Page.loadingStopped
This event will be sent in WebCore::ProgressTracker::finalProgressComplete. No parameters. For now it will only be sent for the main frame.

3) Page.navigationScheduled
This event will be sent in WebCore::NavigationScheduler when a navigation is scheduled. It will include a parameter of how long until the navigation is scheduled to occur. For now it will only be sent for the main frame.

4) Page.noNavigationScheduled
This event will be sent in WebCore::NavigationScheduler AFTER either a scheduled navigation is canceled (and there's no replacement navigation) or a scheduled navigation has been processed. By processed I mean after the WebCore::ScheduledNavigation::Fire method has returned. For many types of navigations, this will result in a Page.loadingStart before Page.noNavigationScheduled. Some navigations may not start synchronously with respect to the Fire method (for example, history.back() in the chromium port). In the future, perhaps we could improve the code to handle those cases too. However, I think this would be quite difficult and would require scattering the codebase (within WebCore and the port) with instrumentation calls every time we drop a navigation we were considering. Regardless of whether we improve this in the future, this event as proposed would still catch most navigation cases and I think would be useful. For now it will only be sent for the main frame.


Here's an example of how a inspector client could use these events to track page navigations:

1) Client sends imaginary dispatchMouseEvent command to click a button that will submit a form which will cause a navigation.
2) Processing of the click causes a new navigation to be scheduled in WebCore::NavigationScheduler.
3) The Page.navigationScheduled event is sent.
4) Processing of the click finishes and the command response for dispatchMouseEvent is sent.
5) Client sees the Page.navigationScheduled event before the command response and decides to wait for the scheduled navigation to occur.
6) The timer for the scheduled navigation fires in WebCore::NavigationScheduler, and the WebCore::ScheduledNavigation::Fire is called.

option A - WebCore decides not to navigate because the page's onsubmit script said no.
7) The WebCore::ScheduledNavigation::Fire method finishes and WebCore::NavigationScheduler sends Page.noNavigationScheduled.
8) The client sees the Page.noNavigationScheduled and stops waiting for the canceled navigation.

option B - WebCore decides to navigate
7) Page.loadingStarted is sent before WebCore::ScheduledNavigation::Fire returns.
8) The WebCore::ScheduledNavigation::Fire method finishes and WebCore::NavigationScheduler sends Page.noNavigationScheduled.
9) The client sees that loading has started, and decides to wait for that to finish, even though it sees there are no more scheduled navigations.


If you want to discuss names, here's some other ones I considered for Page.navigationScheduled:
Page.aboutToNavigate
Page.mayNavigate
Page.navigationPlanned
Page.consideringNavigation
Page.potentialNavigation
Page.newPotentialNavigation
Page.newPendingNavigation

[Ken Kania]

Does this sound reasonable?

[Ken Kania]

Created attachment 178310 [details]
Patch

[WebKit Review Bot]

Comment on attachment 178310 [details]
Patch

Attachment 178310 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15218030

[Peter Beverloo (cr-android ews)]

Comment on attachment 178310 [details]
Patch

Attachment 178310 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15212336

[Ken Kania]

Created attachment 178660 [details]
Patch

[Ken Kania]

I changed a few things from the original proposal. First, I send the events for all frames, along with the frame info. I was planning on adding this later, but it was simple enough and it simplified the tests. Second, I used different event names than proposed to try to match the frameNavigated and frameDetached events.

[WebKit Review Bot]

Comment on attachment 178660 [details]
Patch

Attachment 178660 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15257463

[Build Bot]

Comment on attachment 178660 [details]
Patch

Attachment 178660 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15276417

New failing tests:
http/tests/inspector/appcache/appcache-iframe-manifests.html
http/tests/inspector/resource-main-cookies.php
http/tests/inspector/extensions-network-redirect.html
http/tests/inspector/resource-tree/resource-request-content-while-loading.html
http/tests/inspector/change-iframe-src.html
http/tests/inspector/network/x-frame-options-deny.html
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/console-resource-errors.html
http/tests/inspector/resource-tree/resource-tree-errors-reload.html
inspector/audits/audits-panel-functional.html
http/tests/inspector/resource-tree/resource-tree-non-unique-url.html
http/tests/inspector/network/network-initiator-from-console.html
http/tests/inspector/extensions-useragent.html
http/tests/inspector/appcache/appcache-swap.html
http/tests/inspector/network/network-iframe-load-and-delete.html
http/tests/inspector/network/download.html
inspector/console/alert-toString-exception.html
inspector/debugger/debugger-autocontinue-on-syntax-error.html
http/tests/inspector/network/network-initiator.html
http/tests/inspector/extensions-ignore-cache.html
fast/loader/javascript-url-in-object.html
http/tests/inspector/resource-har-pages.html
fast/history/history-subframe-with-name.html
inspector/audits/audits-panel-noimages-functional.html

[Ken Kania]

Created attachment 179527 [details]
Patch

[Pavel Feldman]

Comment on attachment 179527 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179527&action=review

> Source/WebCore/inspector/Inspector.json:494
> +                    { "name": "frame", "$ref": "Frame", "description": "Frame object." }

Originally, we were only sending the frame information once in frameNavigated and then operated frameId only. Is there a predefined order of appearance of frameNavigated / frameStartedLoading / frameScheduledNavigation? It would be great if we could send frame with the first one and operate frameId with the latter.

[Ken Kania]

(In reply to comment #10)
> (From update of attachment 179527 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179527&action=review
> 
> > Source/WebCore/inspector/Inspector.json:494
> > +                    { "name": "frame", "$ref": "Frame", "description": "Frame object." }
> 
> Originally, we were only sending the frame information once in frameNavigated and then operated frameId only. Is there a predefined order of appearance of frameNavigated / frameStartedLoading / frameScheduledNavigation? It would be great if we could send frame with the first one and operate frameId with the latter.

frameStartedLoading should come first, I think in all circumstances. However, when the frame starts loading, many of the Frame characteristics (e.g., url) are from the previous navigation. It would be fine to only pass the full Frame on frameNavigated and leave the others as just frameId. Sound good?

[Ken Kania]

Created attachment 179959 [details]
Patch

[Pavel Feldman]

Comment on attachment 179959 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=179959&action=review

Please fix the naming prior to landing.

> Source/WebCore/inspector/Inspector.json:494
> +                    { "name": "frame", "$ref": "Network.FrameId", "description": "Id of the frame that has started loading." }

You should name it frameId

[Ken Kania]

Created attachment 180001 [details]
Patch

[Peter Beverloo (cr-android ews)]

Comment on attachment 180001 [details]
Patch

Attachment 180001 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15400311

[Ken Kania]

Created attachment 180065 [details]
Patch

[Peter Beverloo (cr-android ews)]

Comment on attachment 180065 [details]
Patch

Attachment 180065 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15408414

[WebKit Review Bot]

Comment on attachment 180065 [details]
Patch

Attachment 180065 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15406500

[WebKit Review Bot]

Comment on attachment 180065 [details]
Patch

Rejecting attachment 180065 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
t-queue/Source/WebKit/chromium/third_party/smhasher/src --revision 147 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
49>At revision 147.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/15403740

[Ken Kania]

Created attachment 181756 [details]
Patch

[WebKit Review Bot]

Comment on attachment 181756 [details]
Patch

Attachment 181756 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15758273

[Ken Kania]

Created attachment 181799 [details]
Patch

[WebKit Review Bot]

Comment on attachment 181799 [details]
Patch

Attachment 181799 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15766242

[Peter Beverloo (cr-android ews)]

Comment on attachment 181799 [details]
Patch

Attachment 181799 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15757342

[Ken Kania]

Created attachment 182801 [details]
Patch

[WebKit Review Bot]

Comment on attachment 182801 [details]
Patch

Clearing flags on attachment: 182801

Committed r139853: <http://trac.webkit.org/changeset/139853>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 107049

[Levi Weintraub]

Apologies for the rollout. This ppapi_unittests to fail to compile on ChromeOS builders.

Logs: http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74823/steps/ppapi_unittests/logs/stdio

From the logs:
/mnt/scratch0/b_used/build/slave/linux_chromeos/build/src/build/../sconsbuild/Release/ppapi_unittests: symbol lookup error: /mnt/scratch0/b_used/build/slave/linux_chromeos/build/src/out/Release/lib.target/libwebkit.so: undefined symbol: _ZN7WebCore17InspectorFrontend4Page31frameClearedScheduledNavigationERKN3WTF6StringE

[Ken Kania]

(In reply to comment #29)
> Apologies for the rollout. This ppapi_unittests to fail to compile on ChromeOS builders.
> 
> Logs: http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74823/steps/ppapi_unittests/logs/stdio
> 
> From the logs:
> /mnt/scratch0/b_used/build/slave/linux_chromeos/build/src/build/../sconsbuild/Release/ppapi_unittests: symbol lookup error: /mnt/scratch0/b_used/build/slave/linux_chromeos/build/src/out/Release/lib.target/libwebkit.so: undefined symbol: _ZN7WebCore17InspectorFrontend4Page31frameClearedScheduledNavigationERKN3WTF6StringE

Sorry for making a mess!

I was able to reproduce these build failures (see my try job http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74956). It seems for some reason it isn't regenerating InspectorFrontend.cpp. I've noticed the webkit ews system also has sometimes not been able to build, I suppose for a similar reason. The build/tests work fine when I submitted the same job with clobber=True (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74982).

I'll take a look and see if I can find any problems with how the dependencies are configured.

[Ken Kania]

(In reply to comment #30)
> (In reply to comment #29)
> > Apologies for the rollout. This ppapi_unittests to fail to compile on ChromeOS builders.
> > 
> > Logs: http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74823/steps/ppapi_unittests/logs/stdio
> > 
> > From the logs:
> > /mnt/scratch0/b_used/build/slave/linux_chromeos/build/src/build/../sconsbuild/Release/ppapi_unittests: symbol lookup error: /mnt/scratch0/b_used/build/slave/linux_chromeos/build/src/out/Release/lib.target/libwebkit.so: undefined symbol: _ZN7WebCore17InspectorFrontend4Page31frameClearedScheduledNavigationERKN3WTF6StringE
> 
> Sorry for making a mess!
> 
> I was able to reproduce these build failures (see my try job http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74956). It seems for some reason it isn't regenerating InspectorFrontend.cpp. I've noticed the webkit ews system also has sometimes not been able to build, I suppose for a similar reason. The build/tests work fine when I submitted the same job with clobber=True (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromeos/builds/74982).
> 
> I'll take a look and see if I can find any problems with how the dependencies are configured.

I was able to repro the build problem locally with make. I've filed http://code.google.com/p/gyp/issues/detail?id=318

[Ken Kania]

Created attachment 184034 [details]
Patch

[Pavel Feldman]

Comment on attachment 184034 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184034&action=review

> Source/WebCore/inspector/Inspector.json:3401
> +                    { "name": "x", "type": "integer", "description": "X coordinate of the event relative to the main frame's viewport."},

Why is this changing here?

[Ken Kania]

(In reply to comment #33)
> (From update of attachment 184034 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184034&action=review
> 
> > Source/WebCore/inspector/Inspector.json:3401
> > +                    { "name": "x", "type": "integer", "description": "X coordinate of the event relative to the main frame's viewport."},
> 
> Why is this changing here?

It's minor cleanup and it has the good side effect of bypassing the gyp problem.

[WebKit Review Bot]

Comment on attachment 184034 [details]
Patch

Rejecting attachment 184034 [details] from commit-queue.

[e-mail] does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.

[WebKit Review Bot]

Comment on attachment 184034 [details]
Patch

Clearing flags on attachment: 184034

Committed r140649: <http://trac.webkit.org/changeset/140649>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Andrey Adaikin]

(In reply to comment #34)
> (In reply to comment #33)
> > (From update of attachment 184034 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=184034&action=review
> > 
> > > Source/WebCore/inspector/Inspector.json:3401
> > > +                    { "name": "x", "type": "integer", "description": "X coordinate of the event relative to the main frame's viewport."},
> > 
> > Why is this changing here?
> 
> It's minor cleanup and it has the good side effect of bypassing the gyp problem.

FYI. JSComiler argues with you:

Source/WebCore/inspector/front-end/protocol-externs.js:2523: WARNING - optional arguments must be at the end
InputAgent.dispatchMouseEvent = function(type, opt_modifiers, opt_timestamp, x, y, opt_button, opt_clickCount, opt_callback) {}
                                ^

0 error(s), 2 warning(s), 80.4% typed
Compiling InjectedScriptSource.js...
Source/WebCore/inspector/front-end/protocol-externs.js:2523: WARNING - optional arguments must be at the end
InputAgent.dispatchMouseEvent = function(type, opt_modifiers, opt_timestamp, x, y, opt_button, opt_clickCount, opt_callback) {}
                                ^

0 error(s), 1 warning(s), 90.0% typed
Compiling InjectedScriptCanvasModuleSource.js...
Source/WebCore/inspector/front-end/protocol-externs.js:2523: WARNING - optional arguments must be at the end
InputAgent.dispatchMouseEvent = function(type, opt_modifiers, opt_timestamp, x, y, opt_button, opt_clickCount, opt_callback) {}

[Ken Kania]

(In reply to comment #38)
> (In reply to comment #34)
> > (In reply to comment #33)
> > > (From update of attachment 184034 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=184034&action=review
> > > 
> > > > Source/WebCore/inspector/Inspector.json:3401
> > > > +                    { "name": "x", "type": "integer", "description": "X coordinate of the event relative to the main frame's viewport."},
> > > 
> > > Why is this changing here?
> > 
> > It's minor cleanup and it has the good side effect of bypassing the gyp problem.
> 
> FYI. JSComiler argues with you:
> 
> Source/WebCore/inspector/front-end/protocol-externs.js:2523: WARNING - optional arguments must be at the end
> InputAgent.dispatchMouseEvent = function(type, opt_modifiers, opt_timestamp, x, y, opt_button, opt_clickCount, opt_callback) {}
>                                 ^
> 
> 0 error(s), 2 warning(s), 80.4% typed
> Compiling InjectedScriptSource.js...
> Source/WebCore/inspector/front-end/protocol-externs.js:2523: WARNING - optional arguments must be at the end
> InputAgent.dispatchMouseEvent = function(type, opt_modifiers, opt_timestamp, x, y, opt_button, opt_clickCount, opt_callback) {}
>                                 ^
> 
> 0 error(s), 1 warning(s), 90.0% typed
> Compiling InjectedScriptCanvasModuleSource.js...
> Source/WebCore/inspector/front-end/protocol-externs.js:2523: WARNING - optional arguments must be at the end
> InputAgent.dispatchMouseEvent = function(type, opt_modifiers, opt_timestamp, x, y, opt_button, opt_clickCount, opt_callback) {}

Ah, forgot about the frontend JS func. I've moved the params back to their original place in https://bugs.webkit.org/show_bug.cgi?id=107828. Please take a look.

[Pavel Feldman]

> Ah, forgot about the frontend JS func. I've moved the params back to their original place in https://bugs.webkit.org/show_bug.cgi?id=107828. Please take a look.

You should file a new bug.