Bug 62894 – Web Inspector: backend needs to provide system-unique object ids, so these remain unique across navigation

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

[Andrey Kosyakov]

For multi-process browsers, inspector front-end may communicate with agents residing in various processes, e.g. in case of navigation. To avoid necessity to clean up all object maps upon navigation and simplify client implementation, it is suggested that multi-process browsers start providing system-unique object identifiers.

This patch is sort of proof of concept, adding composite identifiers for frames only. This needs eventually be done for other objects (perhaps on the generated protocol stubs level).

[Andrey Kosyakov]

Created attachment 97627 [details]
patch

[WebKit Review Bot]

Attachment 97627 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/inspector/InspectorPageAgent.h:108:  The parameter name "identifier" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrey Kosyakov]

Created attachment 97628 [details]
patch (dropped uninformative parameter names)

[WebKit Review Bot]

Comment on attachment 97628 [details]
patch (dropped uninformative parameter names)

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

New failing tests:
inspector/debugger/open-close-open.html
inspector/styles/styles-history.html

[WebKit Review Bot]

Created attachment 97650 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Andrey Kosyakov]

Comment on attachment 97628 [details]
patch (dropped uninformative parameter names)

As discussed offline, will make the id generation/parsing part of the generated protocol stubs/proxies.

[Andrey Kosyakov]

Created attachment 98904 [details]
patch

[Andrey Kosyakov]

Created attachment 99077 [details]
patch

As discussed offline: we don't want composite ids to be processed on the protocol level, as some objects are built on the application layer (by the back-end)
This is a modification of the first approach -- we just maintain composite string ids for the frames on the back-end as well.

[Pavel Feldman]

Comment on attachment 99077 [details]
patch

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

A couple of nits, otherwise looks ok.

> Source/WebCore/inspector/InspectorController.cpp:352
> +void InspectorController::setAgentProcessIdentifier(unsigned int identifier)

WebCore should not operate 'processes'. Please come up with a neutral name (AgentUniqueIdentifier?). Also it should probably be a string.

[Andrey Kosyakov]

Created attachment 99088 [details]
patch

Renamed AgentProcessIdentifier to AgentIdentifierPrefix, changed it to string.

[WebKit Review Bot]

Attachment 99088 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/inspector/InspectorController.h:96:  The parameter name "prefix" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Andrey Kosyakov]

Manually committed r90017: http://trac.webkit.org/changeset/90017