Bug 104168 - Web Inspector: Add events for tracking when a page is about to start loading or is loading
Summary: Web Inspector: Add events for tracking when a page is about to start loading ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Ken Kania
URL:
Keywords:
Depends on: 107049
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-05 13:53 PST by Ken Kania
Modified: 2013-01-24 08:51 PST (History)
14 users (show)

See Also:


Attachments
Patch (14.60 KB, patch)
2012-12-07 17:10 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (19.53 KB, patch)
2012-12-10 16:04 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.75 KB, patch)
2012-12-14 13:34 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.82 KB, patch)
2012-12-18 09:24 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.83 KB, patch)
2012-12-18 12:19 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.85 KB, patch)
2012-12-18 17:20 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.86 KB, patch)
2013-01-08 14:07 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.86 KB, patch)
2013-01-08 16:24 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (20.79 KB, patch)
2013-01-15 10:11 PST, Ken Kania
no flags Details | Formatted Diff | Diff
Patch (27.40 KB, patch)
2013-01-22 12:22 PST, Ken Kania
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ken Kania 2012-12-05 13:53:20 PST
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
Comment 1 Ken Kania 2012-12-05 13:54:02 PST
Does this sound reasonable?
Comment 2 Ken Kania 2012-12-07 17:10:27 PST
Created attachment 178310 [details]
Patch
Comment 3 WebKit Review Bot 2012-12-07 18:34:27 PST
Comment on attachment 178310 [details]
Patch

Attachment 178310 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15218030
Comment 4 Peter Beverloo (cr-android ews) 2012-12-07 18:34:44 PST
Comment on attachment 178310 [details]
Patch

Attachment 178310 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15212336
Comment 5 Ken Kania 2012-12-10 16:04:25 PST
Created attachment 178660 [details]
Patch
Comment 6 Ken Kania 2012-12-10 16:07:01 PST
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.
Comment 7 WebKit Review Bot 2012-12-10 21:21:13 PST
Comment on attachment 178660 [details]
Patch

Attachment 178660 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15257463
Comment 8 Build Bot 2012-12-11 15:49:23 PST
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
Comment 9 Ken Kania 2012-12-14 13:34:00 PST
Created attachment 179527 [details]
Patch
Comment 10 Pavel Feldman 2012-12-18 05:24:27 PST
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.
Comment 11 Ken Kania 2012-12-18 09:11:28 PST
(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?
Comment 12 Ken Kania 2012-12-18 09:24:54 PST
Created attachment 179959 [details]
Patch
Comment 13 Pavel Feldman 2012-12-18 09:41:03 PST
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
Comment 14 Ken Kania 2012-12-18 12:19:13 PST
Created attachment 180001 [details]
Patch
Comment 15 Peter Beverloo (cr-android ews) 2012-12-18 13:48:23 PST
Comment on attachment 180001 [details]
Patch

Attachment 180001 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15400311
Comment 16 Ken Kania 2012-12-18 17:20:25 PST
Created attachment 180065 [details]
Patch
Comment 17 Peter Beverloo (cr-android ews) 2012-12-18 17:53:32 PST
Comment on attachment 180065 [details]
Patch

Attachment 180065 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15408414
Comment 18 WebKit Review Bot 2012-12-18 23:39:50 PST
Comment on attachment 180065 [details]
Patch

Attachment 180065 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15406500
Comment 19 WebKit Review Bot 2012-12-19 08:10:46 PST
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
Comment 20 Ken Kania 2013-01-08 14:07:25 PST
Created attachment 181756 [details]
Patch
Comment 21 WebKit Review Bot 2013-01-08 14:56:11 PST
Comment on attachment 181756 [details]
Patch

Attachment 181756 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15758273
Comment 22 Ken Kania 2013-01-08 16:24:34 PST
Created attachment 181799 [details]
Patch
Comment 23 WebKit Review Bot 2013-01-08 18:53:53 PST
Comment on attachment 181799 [details]
Patch

Attachment 181799 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15766242
Comment 24 Peter Beverloo (cr-android ews) 2013-01-08 19:34:51 PST
Comment on attachment 181799 [details]
Patch

Attachment 181799 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15757342
Comment 25 Ken Kania 2013-01-15 10:11:42 PST
Created attachment 182801 [details]
Patch
Comment 26 WebKit Review Bot 2013-01-16 01:51:44 PST
Comment on attachment 182801 [details]
Patch

Clearing flags on attachment: 182801

Committed r139853: <http://trac.webkit.org/changeset/139853>
Comment 27 WebKit Review Bot 2013-01-16 01:51:50 PST
All reviewed patches have been landed.  Closing bug.
Comment 28 WebKit Review Bot 2013-01-16 13:32:25 PST
Re-opened since this is blocked by bug 107049
Comment 29 Levi Weintraub 2013-01-16 13:38:16 PST
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
Comment 30 Ken Kania 2013-01-17 09:43:58 PST
(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.
Comment 31 Ken Kania 2013-01-18 09:30:18 PST
(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
Comment 32 Ken Kania 2013-01-22 12:22:51 PST
Created attachment 184034 [details]
Patch
Comment 33 Pavel Feldman 2013-01-23 04:49:13 PST
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?
Comment 34 Ken Kania 2013-01-23 08:38:49 PST
(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.
Comment 35 WebKit Review Bot 2013-01-23 08:58:52 PST
Comment on attachment 184034 [details]
Patch

Rejecting attachment 184034 [details] from commit-queue.

kkania@chromium.org 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.
Comment 36 WebKit Review Bot 2013-01-23 21:44:03 PST
Comment on attachment 184034 [details]
Patch

Clearing flags on attachment: 184034

Committed r140649: <http://trac.webkit.org/changeset/140649>
Comment 37 WebKit Review Bot 2013-01-23 21:44:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 38 Andrey Adaikin 2013-01-24 05:48:05 PST
(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) {}
Comment 39 Ken Kania 2013-01-24 08:14:16 PST
(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.
Comment 40 Pavel Feldman 2013-01-24 08:51:30 PST
> 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.