Question about how to add new inspector instrumentation

[Connor Clark]

Quick background: Lighthouse is adding a new audit for form autofill. Chrome has a prediction system for what input elements should use for an "autocomplete" value. This prediction system is quite useful, and we want to surface these suggestions in Lighthouse.  There's a hacky way to get this data via `--enable-features=AutofillShowTypePredictions`, which tosses a ton of data in a DOM element attribute. This works enough for us to experiment with, but we really need protocol support.

So I've been exploring how to expose this prediction data to InspectorDomAgent.

The autofill prediction system is here. I attempted to add a new core probe here, but I ran into various issues that made me think this wasn't the right approach. Then I noticed literally nothing inside "components/" uses probes, which seems to confirm that's the wrong approach. Unfortunately, I have no idea how else to approach this.

Are there any examples of a DevTools inspector agent getting data from some component in "components/"? Is it possible?

[Simon Zünd]

I had a quick look and it seems the autofill component is set-up as a TabHelper on the browser process side. Can you confirm this?

core probes are the mechanism used, to instrument things in the renderer process. Everything in the browser process is inspected via devtools_instrumentation.h. "content/browser/devtools" also contains the browser process implementation of the various CDP domains. E.g. for the DOM domain, whereas the renderer implements them in "inspector_dom_agent.cc", you will find them in "dom_handler.cc" on the browser process side.

If the autofill component is part of "chrome" not "content" then there is also "chrome/browser/devtools" that mirrors what's going on in "content". We'd probably have to hookup some kind of observer that monitors the autofill component, with "chrome/browser/devtools" or "content/browser/devtools".

Cheers, Simon

--
You received this message because you are subscribed to the Google Groups "devtools-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to devtools-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/devtools-dev/CAEFKyh4vJHXh3vpMFjmh_rieThrgZAy9Hu-p2GGwr56oVzMBtQ%40mail.gmail.com.

[Yang Guo]

Does it make sense to surface autofillability in DevTools as well? E.g. via adorner in the DOM tree and the popover overlay?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/devtools-dev/CACswSC1u_v-w%2Bx9giQUXTsmOgJiDnULJaz9pem8f%2BqhRaUB2Fg%40mail.gmail.com.

[Connor Clark]

> I had a quick look and it seems the autofill component is set-up as a TabHelper on the browser process side. Can you confirm this?

I don't believe it is. There are a couple tab helpers defined in components/autofill, but they are only for ios form filling. Also nothing mentioning autofill in "TabHelpers::AttachTabHelpers". There is a single tab helper attached dealing with form interaction/activity, but I don't think it is related.

I think "components/autofill" is probably utilized by some tab helpers?

> If the autofill component is part of "chrome" not "content" then there is also "chrome/browser/devtools" that mirrors what's going on in "content". We'd probably have to hookup some kind of observer that monitors the autofill component, with "chrome/browser/devtools" or "content/browser/devtools".

I see a number of items in "chrome/browser" use this component (aka they call this method that returns an autofill driver for a given RenderFrameHost).

Looking at chrome/browser/devtools/protocol/page_handler.cc impl of GetInstallabilityErrors, I'm seeing how I might be able to get a reference to the autofill driver. Perhaps this is the correct place to add an observer in the autofill driver. I think the part of autofill that should be hooked into is AutofillManager::OnLoadedServerPredictions. Not sure where to begin (I'm not really a Chromium C++ contributor), but perhaps I could manage it given an example of a similar bit of code...any ideas?

> Does it make sense to surface autofillability in DevTools as well? E.g. via adorner in the DOM tree and the popover overlay?

I think it'd be useful adorning an input element that lacks an autocomplete value and Chrome has an opinion on what it should be, yes.

[Simon Zünd]

Looking at chrome/browser/devtools/protocol/page_handler.cc impl of GetInstallabilityErrors, I'm seeing how I might be able to get a reference to the autofill driver. Perhaps this is the correct place to add an observer in the autofill driver. I think the part of autofill that should be hooked into is AutofillManager::OnLoadedServerPredictions. Not sure where to begin (I'm not really a Chromium C++ contributor), but perhaps I could manage it given an example of a similar bit of code...any ideas?

AFAICT, the AutofillManager passes the calculated predictions in "OnLoadedServerPredictions" via this "autofill_client.h" interface to the "chrome/browser" module and it ends up in chrome_autofill_client. Assuming those are the correct predictions then we could instrument there. I'd suggest the following approach to go forward:

1. Build a "quick" prototype using the steps described below
2. Write it up in a small DevTools Design Doc
3. Get feedback from an autofill component owner and DevTools backend owner (I suggest caseq@ or sigurds@ for DevTools)
4. And iterate based on feedback

I'd suggest to build the prototype using the following approach (taken with a grain of salt, as I only looked at the code for a whole 10 minutes):

• Add a new Event to CDP (browser_protocol.pdl). Something like "onAutofillPredictions". Probably fits best into the "Page" domain.
• Implement a method in chrome/browser/devtools/page_handler.cc that takes the autofill component predictions and turns it into the new CDP Event
• Instrument ChromeAutofillClient::PropagateAutofillPredictions to call into page_handler. You can check in content/browser/devtools/devtools_instrumentation.cc how to dispatch to the right handler from a single static method.
• Alternatively to the step above: We probably need our own devtools_instrumentation.h in chrome/browser, or turn the one in content/ into a public API. We can discuss that in the Design Doc though.

Hope this helps. Feel free to ping me in chat directly if you get stuck during implementation.

Cheers, Simon

[Yang Guo]

Please consider possible use cases outside of Lighthouse when designing the new CDP event. Thanks!

Cheers,

Yang

[Andrey Kosyakov]

Hi Connor,

here are some notes to help you with the design. As you noticed, the only place where our handy "probe" machinery is available is blink/core, which is not applicable in case of autofill, since it's a content-level component. It's also not available in the browser, as Simon rightfully stated, but given that you're ultimately dealing with DOM nodes and their properties, the best place to instrument would be renderer, anyway. Note that the data of interest are ultimately processed in the render.

This probably deserves a CDP domain and an agent of its own, given the "component" nature of autofill (case in point: some browsers don't have autofill, headless is one example; consider whether the implications of this on availability of the feature in Lightrider is critical for you).

One other thing that may further complicate the implementation is that we don't have any instrumentation points in the content/renderer right now, but, fortunately, autofill has also an associated blink module that may be (ab)used to wire the instrumentation to DevTools.

+1 to Yang's point of considering usages beyond Lighthouse -- my feeling is that this may actually be quite valuable as an authoring phase feature and could probably be exposed in the DevTools overlay, which is a way better UX than the "feature" we currently expose (this would hopefully also justify the additional plumbing required for instrumentation to the Autofill owners).

If you need an example of a somewhat similar functionality in blink modules, take a look at InspectorAccessibilityAgent or InspectorWebAudioAgent.

Best regards,

Andrey.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/devtools-dev/CAFSTc_h-VvoHD6n7otK4-b0VUZ7Qujh-99yEMGfBKoJ88NuZxQ%40mail.gmail.com.

[Connor Clark]

Thanks for the advice!

Coming back to this now, spent the last day trying to navigate Chromium. I tried both suggestions (instrumenting from content/browser and instrumenting from renderer), but couldn't get either to work. https://chromium-review.googlesource.com/c/chromium/src/+/2618698

> Instrument ChromeAutofillClient::PropagateAutofillPredictions to call into page_handler. You can check in content/browser/devtools/devtools_instrumentation.cc how to dispatch to the right handler from a single static method.

When I tried this, I ultimately got this error:

"""

ld.lld: error: undefined symbol: content::protocol::AutofillHandler::ForAgentHost(content::DevToolsAgentHostImpl*)

>>> referenced by chrome_autofill_client.cc

>>>               ui/chrome_autofill_client.o:(autofill::ChromeAutofillClient::PropagateAutofillPredictions(content::RenderFrameHost*, std::__Cr::vector<autofill::FormStructure*, std::__Cr::allocator<autofill::FormStructure*> > const&)) in archive obj/chrome/browser/ui/libui.a

ld.lld: error: undefined symbol: content::protocol::AutofillHandler::OnAutofillPredictions()

>>> referenced by chrome_autofill_client.cc

>>>               ui/chrome_autofill_client.o:(autofill::ChromeAutofillClient::PropagateAutofillPredictions(content::RenderFrameHost*, std::__Cr::vector<autofill::FormStructure*, std::__Cr::allocator<autofill::FormStructure*> > const&)) in archive obj/chrome/browser/ui/libui.a

"""

I assume it is because I included "content/browser" in "chrome/browser/ui/autofill/chrome_autofill_client.cc"... I noticed nothing in "chrome/browser/ui" includes "content/browser", so it seems wrong.

Perhaps I misunderstood your suggestion for how to dispatch to the correct handler?

> Alternatively to the step above: We probably need our own devtools_instrumentation.h in chrome/browser, or turn the one in content/ into a public API. We can discuss that in the Design Doc though.

Could it be that this is necessary for even a MVP?

> One other thing that may further complicate the implementation is that we don't have any instrumentation points in the content/renderer right now, but, fortunately, autofill has also an associated blink module that may be (ab)used to wire the instrumentation to DevTools.

I can't figure out how to abuse this :)

I tried making my own module "third_party/blink/renderer/modules/autofill" and attaching to the devtools session in "modules_initializer.cc", but I couldn't figure out how to call into it from "components/autofill". I guess the autofill predictions need to go through a public API?

[Andrey Kosyakov]

On Fri, Jan 8, 2021 at 3:43 PM Connor Clark <cja...@google.com> wrote:

Thanks for the advice!

Coming back to this now, spent the last day trying to navigate Chromium. I tried both suggestions (instrumenting from content/browser and instrumenting from renderer), but couldn't get either to work. https://chromium-review.googlesource.com/c/chromium/src/+/2618698

> Instrument ChromeAutofillClient::PropagateAutofillPredictions to call into page_handler. You can check in content/browser/devtools/devtools_instrumentation.cc how to dispatch to the right handler from a single static method.

When I tried this, I ultimately got this error:

"""

ld.lld: error: undefined symbol: content::protocol::AutofillHandler::ForAgentHost(content::DevToolsAgentHostImpl*)

>>> referenced by chrome_autofill_client.cc

>>>               ui/chrome_autofill_client.o:(autofill::ChromeAutofillClient::PropagateAutofillPredictions(content::RenderFrameHost*, std::__Cr::vector<autofill::FormStructure*, std::__Cr::allocator<autofill::FormStructure*> > const&)) in archive obj/chrome/browser/ui/libui.a

ld.lld: error: undefined symbol: content::protocol::AutofillHandler::OnAutofillPredictions()

>>> referenced by chrome_autofill_client.cc

>>>               ui/chrome_autofill_client.o:(autofill::ChromeAutofillClient::PropagateAutofillPredictions(content::RenderFrameHost*, std::__Cr::vector<autofill::FormStructure*, std::__Cr::allocator<autofill::FormStructure*> > const&)) in archive obj/chrome/browser/ui/libui.a

"""

I think this particular error is due to a missing export directive, although that doesn't really matter because...

I assume it is because I included "content/browser" in "chrome/browser/ui/autofill/chrome_autofill_client.cc"... I noticed nothing in "chrome/browser/ui" includes "content/browser", so it seems wrong.

 ... yes, something from chrome/browser can only talk to content through the content/public API.

> One other thing that may further complicate the implementation is that we don't have any instrumentation points in the content/renderer right now, but, fortunately, autofill has also an associated blink module that may be (ab)used to wire the instrumentation to DevTools.

I can't figure out how to abuse this :)

I tried making my own module "third_party/blink/renderer/modules/autofill" and attaching to the devtools session in "modules_initializer.cc", but I couldn't figure out how to call into it from "components/autofill". I guess the autofill predictions need to go through a public API?

Yes, but a module's public API. There's one already for autofill, check this out: https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/public/web/modules/autofill/web_form_element_observer.h;bpv=1;bpt=1

Best regards,

Andrey.

[Adam Raine]

Update: We got a basic solution working https://chromium-review.googlesource.com/c/chromium/src/+/2630327.

It wires the autofill predictions through the existing autofill blink module to any inspector autofill agents (Defined in third_party/blink/renderer/core/inspector/inspector_autofill_agent.h) enabled on the frame.