Bug 51485 – Web Inspector: support overriding user agent strings

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

[Andrey Kosyakov]

This patch only covers back-end functionality and an extension API.
Note we expose an ability to override user agent to extensions via webInspector.inspectedWindow.reload() to avoid possible conflicts between different extensions trying to override user agent (or rather make these conflicts more explicit).
There will be a separate issue for exposing user agent selection in front-end UI.

[Andrey Kosyakov]

Created attachment 77243 [details]
patch

[Yury Semikhatsky]

Comment on attachment 77243 [details]
patch

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

> WebCore/loader/FrameLoader.cpp:2532
> +        String userAgentOverride = page->inspectorController()->userAgentOverride();

You should use InspectorInstrumentation to access inspector code instead of calling IC directly.

[Pavel Feldman]

> 
> You should use InspectorInstrumentation to access inspector code instead of calling IC directly.

I disagree - inspectorInstrumentation interface is only used for _instrumentation_. Until InspectorController is removed from the page, we are free to use it.

[Yury Semikhatsky]

(In reply to comment #3)
> > 
> > You should use InspectorInstrumentation to access inspector code instead of calling IC directly.
> 
> I disagree - inspectorInstrumentation interface is only used for _instrumentation_. Until InspectorController is removed from the page, we are free to use it.

IMO it's a good example of inspector instrumentation and if we have decided to eventually leave InspectorInstrumentation as the only interface to the inspector component we shouldn't add new direct calls to InspectorController, otherwise I don't see what's the point in having additional intermediate layer(InspectorInstrumentation) which does part of IC work in some cases.

[Pavel Feldman]

> 
> IMO it's a good example of inspector instrumentation and if we have decided to eventually leave InspectorInstrumentation as the only interface to the inspector component we shouldn't add new direct calls to InspectorController, otherwise I don't see what's the point in having additional intermediate layer(InspectorInstrumentation) which does part of IC work in some cases.

Yes and no: we should have InspectorControllerForWebCore insterface. It is not clear whether it should be InspectorInstrumentation (which currently used only for instrumentation). My point is that unless inspector contorller is hidden as you suggest, I think it is a better place for overriding the user agent. InspectorInstrumentation is semi-generated as you remember.

[WebKit Review Bot]

http://trac.webkit.org/changeset/77620 might have broken GTK Linux 64-bit Debug

[Ilya Tikhonovsky]

Comment on attachment 77243 [details]
patch

was landed as r77620

[Steve Block]

Hi Andrey, this might be a stupid question, but is there a reason that you didn't update FrameLoader::loadResourceSynchronously() to respect this new User-Agent override?

[Andrey Kosyakov]

(In reply to comment #8)
> Hi Andrey, this might be a stupid question, but is there a reason that you didn't update FrameLoader::loadResourceSynchronously() to respect this new User-Agent override?

The override should also work in case of loadResourceSynchronously() -- the next line after a call to initialRequest.setHTTPUserAgen() is a call to addExtraFieldsToSubresourceRequest(), which should set the agent. Actually, it looks like the former line is redundant, I'm looking to remove this.

Thanks for bringing this up, though -- I went to add a test for synchronous requests, and it turned out that due to a sad omission while manually landing the change we missed to land the test, and as a result the extension support for webInspector.inspectedWindow.reload() was broken by a subsequent change. Raised bug 65476 for that.

[Steve Block]

> The override should also work in case of loadResourceSynchronously()
Makes sense. Thanks for replying!