[Anonymous iframe] Disable autofill [chromium/src : main]

0 views
Skip to first unread message

Yifan Luo (Gerrit)

unread,
Aug 17, 2021, 4:50:54 AM8/17/21
to Arthur Sonzogni, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org

Attention is currently required from: Arthur Sonzogni.

Yifan Luo would like Arthur Sonzogni to review this change.

View Change

[Anonymous iframe] Disable autofill

Bug: 1233858
Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
---
M components/autofill/content/renderer/autofill_agent.cc
M third_party/blink/public/web/web_node.h
M third_party/blink/renderer/core/dom/node.h
M third_party/blink/renderer/core/exported/web_node.cc
M third_party/blink/renderer/core/html/html_iframe_element.h
5 files changed, 9 insertions(+), 1 deletion(-)


To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-MessageType: newchange

Yifan Luo (Gerrit)

unread,
Aug 17, 2021, 4:51:06 AM8/17/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Arthur Sonzogni.

View Change

1 comment:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Tue, 17 Aug 2021 08:50:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Arthur Sonzogni (Gerrit)

unread,
Aug 17, 2021, 5:14:55 AM8/17/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Camille Lamy, Yifan Luo

Attention is currently required from: Yifan Luo.

Yifan Luo has uploaded this change for review.

View Change

[Anonymous iframe] Disable autofill

Bug: 1233858
Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
---
M components/autofill/content/renderer/autofill_agent.cc
M third_party/blink/public/web/web_node.h
M third_party/blink/renderer/core/dom/node.h
M third_party/blink/renderer/core/exported/web_node.cc
M third_party/blink/renderer/core/html/html_iframe_element.h
5 files changed, 9 insertions(+), 1 deletion(-)


To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-MessageType: newchange

Arthur Sonzogni (Gerrit)

unread,
Aug 17, 2021, 5:15:08 AM8/17/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

1 comment:

  • Patchset:

    • Patch Set #6:

      Thanks!

      The current prototype is made from the renderer process. From there you can't easily know if a document is anonymous of not. Indeed, it depends on the whole chain of ancestor (anonymous iframe propagate to every nested iframe) and timing (anonymous attribute it taken into account at the beginning of the navigation that created the document). Currently the document's anonymousness isn't known to the renderer process.

      For instance with:
      ```
      <iframe anonymous=true>
      #document (1)
      <iframe>
      #document (2)
      </iframe>
      </iframe>
      ```

      This patch won't disable autofill for (2) wrongly.

      I was hoping we could hook into the the locations in the browser process where we provide the data and propose a design doc to the Autofill team.

      The AutofillDriver looked promising, especially everywhere where they look for Incognito mode.
      ```
      // Interface that allows Autofill core code to interact with its driver (i.e.,
      // obtain information from it and give information to it). A concrete
      // implementation must be provided by the driver.
      class AutofillDriver {
      ```

      I am going to send an email to Folks working on Autofill in Munich. I am going to ask them what they believe would be the best way to achieve this, so that we can more easily write a design doc about this.

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Tue, 17 Aug 2021 09:14:51 +0000

Yifan Luo (Gerrit)

unread,
Aug 18, 2021, 5:03:05 AM8/18/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Arthur Sonzogni.

View Change

1 comment:

  • Patchset:

    • Patch Set #6:

      Thanks! […]

      Have you sent the Email? Would you mind cc me on the thread?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 09:02:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-MessageType: comment

Arthur Sonzogni (Gerrit)

unread,
Aug 23, 2021, 8:21:38 AM8/23/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

1 comment:

  • Patchset:

    • Patch Set #6:

      Thanks!

      (Removing this from my visibility list), until this got updated. I would like to use the "anonymous" flags from the document, not the one from iframe attribute.

      This would require propagating to the renderer process the RenderFrameHost::anonymous_ flags toward RenderFrame::anonymous_, via the CommitNavigation IPC.

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 6
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Mon, 23 Aug 2021 12:21:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Yifan Luo (Gerrit)

unread,
Sep 3, 2021, 5:13:36 AM9/3/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

View Change

2 comments:

  • Patchset:

    • Patch Set #18:

      Hey Arthur! I got a problem with the test. Could you please help?

  • File chrome/renderer/autofill/password_autofill_agent_browsertest.cc:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 18
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Comment-Date: Fri, 03 Sep 2021 09:13:22 +0000

Arthur Sonzogni (Gerrit)

unread,
Sep 3, 2021, 7:23:01 AM9/3/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

1 comment:

  • File chrome/renderer/autofill/password_autofill_agent_browsertest.cc:

    • The FirstChild here seems to be nullptr. […]

      I think "LoadHTML" simulate a navigation, by creating a URLLoaderClient able to retrieve the resource and pass it to the document by manually calling CommitNavigation.

      However, after that, blink needs to retrieve the content, parse the HTML, parse the script, execute the script. This takes time and the tasks are posted into a queue, but not yet executed when you call GetMainFrame()->FirstFrame().

      So, this would require to wait for those tasks to happen first.
      However there is a second problem. This test is running inside the renderer process and "simulate" what the browser process would have done. However it is very incomplete and the logic about anonymous iframe isn't execute. It means in your test, the document inside the anonymous iframe won't appear as anonymous.

      I think it would be preferable to run a test with both the browser process and the renderer process running.

      Maybe you could find an example to adapt from:
      chrome/browser/password_manager/password_manager_browsertest.cc
      ?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 18
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Fri, 03 Sep 2021 11:22:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Yifan Luo <l...@chromium.org>
Gerrit-MessageType: comment

Yifan Luo (Gerrit)

unread,
Sep 8, 2021, 4:18:37 AM9/8/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

View Change

1 comment:

  • Patchset:

    • Patch Set #21:

      @arthur You might want to have a look when you got free time?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 21
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 08:18:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Antonio Sartori (Gerrit)

unread,
Sep 8, 2021, 5:31:59 AM9/8/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

1 comment:

  • File content/browser/renderer_host/render_frame_host_impl.cc:

    • Patch Set #21, Line 7431: commit_params->anonymous = anonymous_;

      I wonder how this can work, since `anonymous_` knows whether the *current* document is anonymous, while `commit_params->anonymous` wants to know whether the document to commit should be anonymous. I think you should use NavigationRequest::anonumous() to populate this field instead.

      Also, the fact that the CQ passes with this means that we probably need more tests for this.

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 21
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 09:31:44 +0000

Yifan Luo (Gerrit)

unread,
Sep 8, 2021, 6:35:04 AM9/8/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori.

View Change

1 comment:

  • File content/browser/renderer_host/render_frame_host_impl.cc:

    • I wonder how this can work, since `anonymous_` knows whether the *current* document is anonymous, wh […]

      I thought the document's anonymous or not is base on the status of current frame which is the document commit to?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 21
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 10:34:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>
Gerrit-MessageType: comment

Antonio Sartori (Gerrit)

unread,
Sep 8, 2021, 7:02:49 AM9/8/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

1 comment:

  • File content/browser/renderer_host/render_frame_host_impl.cc:

    • I thought the document's anonymous or not is base on the status of current frame which is the docume […]

      The RenderFrameHost is currently rendering the previous document. It might be that the previous document was not anonymous. But if someone set the anonymous flag on the iframe element since then, the document-to-commit should become anonymous.

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 21
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 11:02:36 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>

Arthur Sonzogni (Gerrit)

unread,
Sep 8, 2021, 12:36:59 PM9/8/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Yifan Luo.

View Change

10 comments:

  • Patchset:

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

    • Patch Set #21, Line 4689:

      IN_PROC_BROWSER_TEST_F(PasswordManagerPrerenderBrowserTest,
      DisableOnAnonymousIframe

      Maybe we should create the iframe without anonymous first, check credentials are populate. Then flip iframe.anonymous and check they aren't populate this time.

    • Patch Set #21, Line 4697:

        // Create an iframe and navigate cross-site.
      NavigationObserver iframe_observer(WebContents());
      iframe_observer.SetPathToWaitFor("/password/crossite_iframe_content.html");
      GURL iframe_url = embedded_test_server()->GetURL(
      "abc.foo.com", "/password/crossite_iframe_content.html");
      std::string create_iframe =
      base::StringPrintf("create_iframe('%s');", iframe_url.spec().c_str());
      ASSERT_TRUE(content::ExecuteScriptWithoutUserGesture(RenderFrameHost(),
      create_iframe));
      iframe_observer.Wait();

      Is this step needed to store credentials, or it can be removed?

    • Patch Set #21, Line 4728: ExecuteScriptWithoutUserGesture

      Deprecated (see below)

    • Patch Set #21, Line 4739: ExecuteScriptWithoutUserGestureAndExtractString

      This is deprecated:
      https://source.chromium.org/chromium/chromium/src/+/main:content/public/test/browser_test_utils.h;l=513

      Noawadays, we should use content::EvalJs / ExecJs / JsReplace / etc...

  • File chrome/test/data/password/password_form_in_anonymous_iframe.html:

    • Patch Set #21, Line 22: window.addEventListener("message", receiveMessage, false);

      This test you copy-pasted is very old. It is doing complicated logic with postMessage / domAutomationController, because it doesn't know how to get access to the RenderFrameHost for the iframe outside of content.

      That's actually quite easy:
      ```
      ASSERT_EQUALS(2u, WebContent()->GetAllFrames().size();
      RenderFrameHost top_level_document = WebContents()->GetAllFrames()[0];
      RenderFrameHost anonymous_document = WebContents()->GetAllFrames()[1];
      ```

      And then execute directly anything you want like querying the "username" value:
      ```
      EXPECT_EQ("temp", content::EvalJs(anonymous_document, "document.getElementById("username").value"));
      ```

    • Patch Set #21, Line 1:

      <html>
      <body id="inParent">

      <script>
      function receiveMessage(event) {
      window.domAutomationController.send(event.data);
      }

      function sendMessage(msg) {
      document.getElementById("iframe").contentWindow.postMessage(msg,"*");
      }

      function create_iframe(src) {
      var ifrm = document.createElement("IFRAME");
      ifrm.anonymous = true;
      ifrm.setAttribute("id", "iframe");
      ifrm.setAttribute("name", "iframe");
      ifrm.setAttribute("src", src);
      document.body.appendChild(ifrm);
      }

      window.addEventListener("message", receiveMessage, false);
      </script>

      </body>
      </html>

      Instead of creating a new page. What about loading "/empty.html" and inject any script you want like:

      ```
      content::EvalJs(render_frame_host, JsReplace(R"(
      const iframe = document.createElement("iframe");
      iframe.anonymous = true;
      iframe.src = $1;
      document.body.appendChild(iframe);
      )", iframe_url);
      ```
  • File content/browser/renderer_host/render_frame_host_impl.cc:

    • The RenderFrameHost is currently rendering the previous document. […]

      +1

      Yes, this should be instead:
      ```
      commit_params->anonymous = navigation_request->anonymous()
      ```

  • File content/public/browser/render_frame_host.h:

  • File third_party/blink/renderer/core/dom/document.h:

    • Patch Set #21, Line 2301: bool anonymous_ = false;

      Can this be:
      ```
      const bool anonymous_;
      ```

      instead, to ensure this is a constant property of the document?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 21
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Wed, 08 Sep 2021 16:36:47 +0000

Yifan Luo (Gerrit)

unread,
Sep 9, 2021, 5:21:46 AM9/9/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

View Change

6 comments:

    •   // Create an iframe and navigate cross-site.
      NavigationObserver iframe_observer(WebContents());
      iframe_observer.SetPathToWaitFor("/password/crossite_iframe_content.html");
      GURL iframe_url = embedded_test_server()->GetURL(
      "abc.foo.com", "/password/crossite_iframe_content.html");
      std::string create_iframe =
      base::StringPrintf("create_iframe('%s');", iframe_url.spec().c_str());
      ASSERT_TRUE(content::ExecuteScriptWithoutUserGesture(RenderFrameHost(),
      create_iframe));
      iframe_observer.Wait();

      Is this step needed to store credentials, or it can be removed?

    • I fined it one way previous tests used to add credentials. There might be easier way, let me see.

    • Done

    • This is deprecated: […]

      Done

  • File content/browser/renderer_host/render_frame_host_impl.cc:

    • +1 […]

      Done

  • File content/public/browser/render_frame_host.h:

    • I don't think this is used outside of content/. So this shouldn't be exposed to embedders. […]

      Done

  • File third_party/blink/renderer/core/dom/document.h:

    • Can this be: […]

      Done

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 22
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Thu, 09 Sep 2021 09:21:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>
Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>

Antonio Sartori (Gerrit)

unread,
Sep 9, 2021, 5:55:37 AM9/9/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Arthur Sonzogni, Yifan Luo.

View Change

7 comments:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 23
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Thu, 09 Sep 2021 09:55:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Yifan Luo (Gerrit)

unread,
Sep 10, 2021, 5:06:36 AM9/10/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

Patch set 23:Code-Review +1

View Change

3 comments:

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

  • File content/browser/renderer_host/render_frame_host_impl.h:

    • The diff tool is truly confused. I actually removed `override` in this patch other than added it.

  • File third_party/blink/public/mojom/navigation/navigation_params.mojom:

    • Patch Set #23, Line 484: // Whether this is an anonymous iframe or not.

      I think "Whether this navigation will commit an anonymous document" is more clear

    • Done

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 23
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Fri, 10 Sep 2021 09:06:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Arthur Sonzogni (Gerrit)

unread,
Sep 10, 2021, 5:09:59 AM9/10/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori.

View Change

5 comments:

  • File chrome/browser/password_manager/password_manager_browsertest.cc:


    • NavigationObserver iframe_observer(WebContents());
      iframe_observer.SetPathToWaitFor("/password/crossite_iframe_content.html");
      GURL iframe_url = embedded_test_server()->GetURL(

    •       "www.bar.com", "/password/crossite_iframe_content.html");
      EXPECT_TRUE(content::EvalJs(RenderFrameHost(),
      content::JsReplace(R"(


    • const iframe = document.createElement("iframe");

    •       iframe.id = "normal";
      iframe.src = $1;
      document.body.appendChild(iframe);
      )",iframe_url),content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractBool());
      iframe_observer.Wait();

      Why do you need to make one navigation in the iframe exactly?
      Can this step be removed, so that we can proceed directly by "Store a password for autofill later"?

    • Patch Set #23, Line 4745:

        EXPECT_TRUE(content::ExecJs(
      normal_document,
      "var usernameRect = "
      "document.getElementById('username').getBoundingClientRect();",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE));
      int normal_y = content::EvalJs(RenderFrameHost(),
      "usernameRect.top + usernameRect.bottom;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      int normal_x = content::EvalJs(RenderFrameHost(),
      "usernameRect.left + usernameRect.right;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      content::SimulateMouseClickAt(WebContents(), 0,
      blink::WebMouseEvent::Button::kLeft,
      gfx::Point(normal_x / 2, normal_y / 2));

      Maybe:
      ```
      content::SimulateMouseClickOrTapElementWithId(WebContents(),"username");
      ```

    • Patch Set #23, Line 4793:

        EXPECT_TRUE(content::ExecJs(
      anonymous_document,
      "var usernameRect = "
      "document.getElementById('username').getBoundingClientRect();",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE));
      int y = content::EvalJs(RenderFrameHost(),
      "usernameRect.top + usernameRect.bottom;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      int x = content::EvalJs(RenderFrameHost(),
      "usernameRect.left + usernameRect.right;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();

      content::SimulateMouseClickAt(WebContents(), 0,
      blink::WebMouseEvent::Button::kLeft,
      gfx::Point(x / 2, y / 2));

      Same here.

  • File third_party/blink/renderer/core/dom/document_init.h:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 23
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Comment-Date: Fri, 10 Sep 2021 09:09:44 +0000

Antonio Sartori (Gerrit)

unread,
Sep 10, 2021, 5:11:42 AM9/10/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

2 comments:

  • Patchset:

    • Patch Set #23:

      I would suggest to split this CL into two parts: […]

      Unresolving to make this visible.

  • File content/browser/renderer_host/render_frame_host_impl.h:

    • The diff tool is truly confused. I actually removed `override` in this patch other than added it.

      Yes. I meant, can you inline again this getter?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 23
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Fri, 10 Sep 2021 09:11:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>

Yifan Luo (Gerrit)

unread,
Sep 13, 2021, 5:20:47 AM9/13/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

Patch set 24:-Commit-Queue

View Change

3 comments:

    •   EXPECT_TRUE(content::ExecJs(
      normal_document,
      "var usernameRect = "
      "document.getElementById('username').getBoundingClientRect();",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE));
      int normal_y = content::EvalJs(RenderFrameHost(),
      "usernameRect.top + usernameRect.bottom;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      int normal_x = content::EvalJs(RenderFrameHost(),
      "usernameRect.left + usernameRect.right;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      content::SimulateMouseClickAt(WebContents(), 0,
      blink::WebMouseEvent::Button::kLeft,
      gfx::Point(normal_x / 2, normal_y / 2));

    • Maybe: […]

      Looks like we this function cannot successfully get the element from iframes.

  • File content/browser/renderer_host/render_frame_host_impl.h:

    • Yes. […]

      Done

  • File third_party/blink/renderer/core/dom/document_init.h:

    • Done

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 24
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Mon, 13 Sep 2021 09:20:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>
Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>

Yifan Luo (Gerrit)

unread,
Sep 13, 2021, 5:21:19 AM9/13/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

Yifan Luo removed a vote from this change.

View Change

Removed Commit-Queue+1 by Yifan Luo <l...@chromium.org>

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 24
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-MessageType: deleteVote

Yifan Luo (Gerrit)

unread,
Sep 13, 2021, 5:21:25 AM9/13/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

Yifan Luo removed a vote from this change.

View Change

Removed Code-Review+1 by Yifan Luo <l...@chromium.org>

Arthur Sonzogni (Gerrit)

unread,
Sep 13, 2021, 7:00:54 AM9/13/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Yifan Luo.

View Change

15 comments:

    •   // Create an iframe and navigate cross-site.


    • NavigationObserver iframe_observer(WebContents());
      iframe_observer.SetPathToWaitFor("/password/crossite_iframe_content.html");
      GURL iframe_url = embedded_test_server()->GetURL(

    •       "abc.foo.com", "/password/crossite_iframe_content.html");
      std::string create_iframe =
      base::StringPrintf("create_iframe('%s');", iframe_url.spec().c_str());
      ASSERT_TRUE(content::ExecuteScriptWithoutUserGesture(RenderFrameHost(),
      create_iframe));
      iframe_observer.Wait();

    • I fined it one way previous tests used to add credentials. There might be easier way, let me see.

      (waiting for this comment to be resolved, one way or another)

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

    • Patch Set #23, Line 4745:

        EXPECT_TRUE(content::ExecJs(
      normal_document,
      "var usernameRect = "
      "document.getElementById('username').getBoundingClientRect();",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE));
      int normal_y = content::EvalJs(RenderFrameHost(),
      "usernameRect.top + usernameRect.bottom;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      int normal_x = content::EvalJs(RenderFrameHost(),
      "usernameRect.left + usernameRect.right;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      content::SimulateMouseClickAt(WebContents(), 0,
      blink::WebMouseEvent::Button::kLeft,
      gfx::Point(normal_x / 2, normal_y / 2));

    • Looks like we this function cannot successfully get the element from iframes.

      Indeed! I don't see any API taking a RFH instead of a WebContent.

      I guess you can keep your code.

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

    • Patch Set #24, Line 4691: www.foo.com

      Sleevi@ tolkd me using the .test top-level domain is preferable.

      What about replacing "www.foo.com" and "www.abc.com", with:

      • a.test
      • b.test
    • Patch Set #24, Line 4702: create_iframe

      If you address one of my previous comment about not adding:
      file password_form_in_anonymous_iframe.html

      Then, you won't have access to "create_iframe".
      So you can add iframe using:
      ```
      EXPECT_TRUE(content::ExecJs(RenderFrameHost(), content::JsReplace(R"(

    • const iframe = document.createElement("iframe");
    •     iframe.src = $1;
      document.body.appendChild(iframe);
      )", iframe_url)));
      ```
    • Patch Set #24, Line 4703: .spec().c_str()

      This can be omitted.

    • Patch Set #24, Line 4711:

        std::string submit =
      "document.getElementById('username').value = 'temp';"
      "document.getElementById('password').value = 'pa55w0rd';"
      "document.getElementById('submit_button').click();";
      ASSERT_TRUE(content::ExecJs(iframe_document, submit));

      Maybe submit can be inlined?
      ```
      ASSERT_TRUE(content::ExecJs(iframe_document, R"(
      document.getElementById('username').value = 'temp';
      document.getElementById('password').value = 'pa55w0rd';
      document.getElementById('submit_button').click();
      )"));
      ```
    • Patch Set #24, Line 4796:

      Previously, we were using "*Wait*ForElementValue".
      This means, password auto filling may take some time before writing the value.

      Here, this test check they are empty immediately. They may be empty because it did not have time to fill. So it means this may not break if your implementation is removed.

      I think you should wait some time before checking the value to ensure the test is not passing because of this.

      ```
      // Let autofill some time to potentially fill the element's values.
      EXPECT_TRUE(content::ExecJs(
      WebContents(), "new Promise(resolve => setTimeout(resolve, 1000)"));
      ```
  • File chrome/renderer/autofill/password_autofill_agent_browsertest.cc:

    • I think "LoadHTML" simulate a navigation, by creating a URLLoaderClient able to retrieve the resourc […]

      (Addressed)

  • File chrome/test/data/password/password_form_in_anonymous_iframe.html:

    • This test you copy-pasted is very old. […]

      (Addressed)

    • Patch Set #21, Line 1:

      <html>
      <body id="inParent">

      <script>
      function receiveMessage(event) {
      window.domAutomationController.send(event.data);
      }

      function sendMessage(msg) {
      document.getElementById("iframe").contentWindow.postMessage(msg,"*");
      }

      function create_iframe(src) {
      var ifrm = document.createElement("IFRAME");
      ifrm.anonymous = true;
      ifrm.setAttribute("id", "iframe");
      ifrm.setAttribute("name", "iframe");
      ifrm.setAttribute("src", src);
      document.body.appendChild(ifrm);
      }

      window.addEventListener("message", receiveMessage, false);
      </script>

      </body>
      </html>

    • Instead of creating a new page. What about loading "/empty. […]

      Ack

    • Patch Set #21, Line 1:

      <html>
      <body id="inParent">

      <script>
      function receiveMessage(event) {
      window.domAutomationController.send(event.data);
      }

      function sendMessage(msg) {
      document.getElementById("iframe").contentWindow.postMessage(msg,"*");
      }

      function create_iframe(src) {
      var ifrm = document.createElement("IFRAME");
      ifrm.anonymous = true;
      ifrm.setAttribute("id", "iframe");
      ifrm.setAttribute("name", "iframe");
      ifrm.setAttribute("src", src);
      document.body.appendChild(ifrm);
      }

      window.addEventListener("message", receiveMessage, false);
      </script>

      </body>
      </html>

    • Instead of creating a new page. What about loading "/empty. […]

      (waiting for this to be addressed)

  • File third_party/blink/renderer/core/loader/document_loader.h:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 24
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Mon, 13 Sep 2021 11:00:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Yifan Luo (Gerrit)

unread,
Sep 13, 2021, 7:30:58 AM9/13/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org

Attention is currently required from: Antonio Sartori, Yifan Luo.

Yifan Luo uploaded patch set #26 to this change.

View Change

[Anonymous iframe] Disable autofill

This CL aimed to disable username and password autofill for
anonymous iframe. For further info, please follow the design
doc:
https://docs.google.com/document/d/1UHEC10kjxfgNmt-ejGkuxgvv63_-dsEH-DhVF_Sthak/edit?usp=sharing


Bug: 1233858
Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
---
M chrome/browser/password_manager/password_manager_browsertest.cc
A chrome/test/data/password/password_form_in_anonymous_iframe.html
M components/autofill/content/renderer/password_autofill_agent.cc
M content/browser/renderer_host/navigation_controller_impl.cc
M content/browser/renderer_host/navigation_entry_impl.cc
M content/browser/renderer_host/navigation_request.cc
M content/browser/renderer_host/render_frame_host_impl.cc
M content/browser/renderer_host/render_frame_host_impl.h
M content/renderer/render_frame_impl.cc
M third_party/blink/public/mojom/navigation/navigation_params.mojom
M third_party/blink/public/web/web_document.h
M third_party/blink/public/web/web_navigation_params.h
M third_party/blink/renderer/core/dom/document.h
M third_party/blink/renderer/core/dom/document_init.cc
M third_party/blink/renderer/core/dom/document_init.h
M third_party/blink/renderer/core/exported/web_document.cc
M third_party/blink/renderer/core/loader/document_loader.cc
M third_party/blink/renderer/core/loader/document_loader.h
18 files changed, 180 insertions(+), 9 deletions(-)

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 26
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-MessageType: newpatchset

Yifan Luo (Gerrit)

unread,
Sep 13, 2021, 9:49:50 AM9/13/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

Patch set 27:Commit-Queue +1

View Change

6 comments:

  • Commit Message:

    • Could you put a brief description of your patch and reference your design doc? […]

      Done

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

    • Patch Set #23, Line 4745:

        EXPECT_TRUE(content::ExecJs(
      normal_document,
      "var usernameRect = "
      "document.getElementById('username').getBoundingClientRect();",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE));
      int normal_y = content::EvalJs(RenderFrameHost(),
      "usernameRect.top + usernameRect.bottom;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      int normal_x = content::EvalJs(RenderFrameHost(),
      "usernameRect.left + usernameRect.right;",
      content::EXECUTE_SCRIPT_NO_USER_GESTURE)
      .ExtractInt();
      content::SimulateMouseClickAt(WebContents(), 0,
      blink::WebMouseEvent::Button::kLeft,
      gfx::Point(normal_x / 2, normal_y / 2));

    • Indeed! I don't see any API taking a RFH instead of a WebContent. […]

      Done

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

    • Sleevi@ tolkd me using the .test top-level domain is preferable. […]

      Done

    • If you address one of my previous comment about not adding: […]

      I would argue for a html file is better and more understandable than write the js three times in the .cc test file.

    • Done

  • File third_party/blink/renderer/core/loader/document_loader.h:

    • I am wondering why we do not trigger the ASSERT: […]

      No idea. Added anonymous_ to SameSizeAsDocumentLoader

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 27
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Mon, 13 Sep 2021 13:49:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Arthur Sonzogni (Gerrit)

unread,
Sep 13, 2021, 12:34:54 PM9/13/21
to Yifan Luo, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Yifan Luo.

View Change

1 comment:

  • File third_party/blink/renderer/core/loader/document_loader.h:

    • No idea. […]

      This was added to force developers to think about propagating state from one Document to the other when cloned, because of XSLT or javascript-url document commit.

      It looks like you ignored the warning above. Could you not, and instead propagate "anonymous" flag in DocumentLoader::CreateWebNavigationParamsToCloneDocument()
      ?

      It would be good adding a test. However, we can do it in a followup. I opened a bug:
      https://bugs.chromium.org/p/chromium/issues/detail?id=1249063

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 27
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Mon, 13 Sep 2021 16:34:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Yifan Luo (Gerrit)

unread,
Sep 15, 2021, 2:35:43 AM9/15/21
to blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori.

Patch set 27:-Commit-Queue

View Change

3 comments:

    • Unresolving to make this visible.

    • Sorry, I've missed this comment. Sounds a right direction to seperate the CL and put it behind the flag with few questions: 1) Do we already have the flag in browser or I need to create a new one? 2) How many browser tests involved in it? Sounds like it would be better to have another seperate cl for the checking anonymous in the exist tests?

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

    • why EvalJs and not ExecJs? What value is this returning?

    • Done

    • Done

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 27
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 06:35:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Yifan Luo (Gerrit)

unread,
Sep 15, 2021, 4:06:38 AM9/15/21
to alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

View Change

1 comment:

  • File content/renderer/render_frame_impl.cc:

    • I fear there are more places where NavigationParams are created. […]

      This particular code looks to be called by render_frame_impl.cc, btw anonymous in commit_params is default to be false which I do not see worries here

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 08:06:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Antonio Sartori (Gerrit)

unread,
Sep 15, 2021, 4:26:05 AM9/15/21
to Yifan Luo, alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Arthur Sonzogni, Yifan Luo.

View Change

1 comment:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 08:25:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>

Arthur Sonzogni (Gerrit)

unread,
Sep 15, 2021, 4:27:05 AM9/15/21
to Yifan Luo, alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Yifan Luo.

View Change

1 comment:

  • File content/renderer/render_frame_impl.cc:

    • This particular code looks to be called by render_frame_impl. […]

      The autofill code is called only when the Document is associated with a RenderFrame.

      IMO, the missing places where anonymous isn't properly plumbed are for the initial empty document and XSLT/Javascript cloning of the document.

      So in:
      1. FrameLoader::Init()
      2. DocumentLoader::CreateWebNavigationParamsToCloneDocument()

      I think you can let a TODO for (1), until we resolve the initial empty document case. However, you can already fix (2)


      There are other document not associated with a RenderFrame. They don't matter for autofill. However, they matter for the future `document.credentialless`. We will have to find something in the future and keep this in mind.

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 08:26:47 +0000

Yifan Luo (Gerrit)

unread,
Sep 15, 2021, 5:22:30 AM9/15/21
to alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori.

View Change

1 comment:

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 09:22:16 +0000

Antonio Sartori (Gerrit)

unread,
Sep 15, 2021, 10:23:34 AM9/15/21
to Yifan Luo, alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Yifan Luo.

View Change

1 comment:

  • Patchset:

    • Patch Set #23:

      I mean we might need a new flag in browser context in this case?

      Why? The 'anonymous' attribute is parsed only if that flag is enabled. If we don't parse 'anonymous', we never set the 'anonymous' bit, so the browser code does nothing.

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Wed, 15 Sep 2021 14:23:16 +0000

Yifan Luo (Gerrit)

unread,
Sep 20, 2021, 4:05:19 AM9/20/21
to alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori.

View Change

1 comment:

  • Patchset:

    • Patch Set #23:

      Why? The 'anonymous' attribute is parsed only if that flag is enabled. […]

      So what do you mean by "test that the anonymous flag in blink and in the browser always agree" here?

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Comment-Date: Mon, 20 Sep 2021 08:05:06 +0000

Yifan Luo (Gerrit)

unread,
Sep 20, 2021, 4:27:26 AM9/20/21
to alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Antonio Sartori, Arthur Sonzogni.

View Change

2 comments:

  • File chrome/browser/password_manager/password_manager_browsertest.cc:

  • File content/renderer/render_frame_impl.cc:

    • The autofill code is called only when the Document is associated with a RenderFrame. […]

      I am already fixed (2) in the cl. I will add a TODO for FrameLoader::Init()

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Antonio Sartori <antonio...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Comment-Date: Mon, 20 Sep 2021 08:27:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>
Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>

Antonio Sartori (Gerrit)

unread,
Sep 20, 2021, 4:35:29 AM9/20/21
to Yifan Luo, alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Arthur Sonzogni, Yifan Luo.

View Change

1 comment:

  • Patchset:

    • Patch Set #23:

      So what do you mean by "test that the anonymous flag in blink and in the browser always agree" here?

      Sorry, the "anonymous flag" should have been the "anonymous bit" or "anonymous attribute". I'd like to test that we always have `RenderFrameHostImpl::anonymous() == Document::anonymous()`

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Mon, 20 Sep 2021 08:35:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Sartori <antonio...@chromium.org>

Yifan Luo (Gerrit)

unread,
Sep 20, 2021, 4:46:25 AM9/20/21
to alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Attention is currently required from: Arthur Sonzogni, Yifan Luo.

View Change

2 comments:

    • This was added to force developers to think about propagating state from one Document to the other w […]

      Done

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-Attention: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Attention: Yifan Luo <l...@chromium.org>
Gerrit-Comment-Date: Mon, 20 Sep 2021 08:46:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Arthur Sonzogni <arthurs...@chromium.org>

Yifan Luo (Gerrit)

unread,
Sep 20, 2021, 4:46:34 AM9/20/21
to alexmo...@chromium.org, creis...@chromium.org, gavinp...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org, loading...@chromium.org, navigation...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, browser-comp...@chromium.org, ftirelo+au...@chromium.org, tmartino+au...@chromium.org, Antonio Sartori, Tricium, Camille Lamy, Arthur Sonzogni, Chromium LUCI CQ, chromium...@chromium.org

Yifan Luo abandoned this change.

View Change

Abandoned

To view, visit change 3093355. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Ibd5fa303b93b88092a0f335b64b7bd41395a4d94
Gerrit-Change-Number: 3093355
Gerrit-PatchSet: 31
Gerrit-Owner: Yifan Luo <l...@chromium.org>
Gerrit-Reviewer: Arthur Sonzogni <arthurs...@chromium.org>
Gerrit-Reviewer: Yifan Luo <l...@chromium.org>
Gerrit-CC: Antonio Sartori <antonio...@chromium.org>
Gerrit-CC: Camille Lamy <cl...@chromium.org>
Gerrit-MessageType: abandon
Reply all
Reply to author
Forward
0 new messages