DevTools: setExtraHTTPHeaders have no effect for cross-process navigation requests |
|||||||||
Issue descriptionChrome revision: r502853 (might be related to plznavigate?) Repro steps: 1. Use Network.setExtraHTTPHeaders to set a custom http header, e.g. "foo": "bar". 2. Do a cross-process navigation request with Page.navigate 3. Sniff request on the server and explore request headers Expected: the request contains the "foo": "bar" header. Actual: the request does not contain the "foo": "bar" header.
,
Oct 13 2017
,
Oct 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9585805fb2fb347b718fc164d053c4edb6d3debe commit 9585805fb2fb347b718fc164d053c4edb6d3debe Author: Andrey Kosyakov <caseq@chromium.org> Date: Tue Oct 17 19:23:18 2017 DevTools: fix extra HTTP headers not sent on browser navigation request Bug: 767683 Change-Id: Ib0da19e7fc845149b622c19af043760fb0eb2bed Reviewed-on: https://chromium-review.googlesource.com/719582 Commit-Queue: Andrey Kosyakov <caseq@chromium.org> Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Cr-Commit-Position: refs/heads/master@{#509486} [modify] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/content/browser/devtools/protocol/network_handler.cc [modify] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/content/browser/devtools/protocol/network_handler.h [modify] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/content/browser/devtools/protocol_config.json [modify] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/content/browser/devtools/render_frame_devtools_agent_host.cc [add] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/resources/echo-headers.php [add] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/set-extra-http-headers-expected.txt [add] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/third_party/WebKit/LayoutTests/http/tests/inspector-protocol/network/set-extra-http-headers.js [modify] https://crrev.com/9585805fb2fb347b718fc164d053c4edb6d3debe/third_party/WebKit/Source/core/inspector/InspectorNetworkAgent.cpp
,
Oct 20 2017
Is this now fixed and can be closed?
,
Oct 26 2017
We presented Puppeteer (https://github.com/GoogleChrome/puppeteer/) on Chrome DevSummit and now working on its final release. This fix is crucially important for Puppeteer 1.0 launch. It got landed after the branch point; however, it seems to be safe enough to be merged back to 63.
,
Oct 26 2017
,
Oct 26 2017
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Oct 27 2017
Before we approve merge to M63, could you please confirm change is well baked/verified in Canary, having enough automation coverage and safe to merge to M63? Also any other comment to justify the merge as M63 is already promoted to Beta and merge bar is high. Thank you.
,
Oct 27 2017
@govind: the change is well baked and proved to be working. It is a local change, it affects only devtools protocol implementation, and it has a good test coverage. These factors make the change safe to merge. Puppeteer is an important project that helps establishing web platform testing. It's very popular, amassed 17000+ stars on github[1] and dependent upon by 150+ projects[2]. If there's a chance we can have this in, it would be a huge win \o/ [1]: https://github.com/GoogleChrome/puppeteer [2]: https://www.npmjs.com/browse/depended/puppeteer
,
Oct 27 2017
Approving merge to M63 branch 3239 based on comments #5 and #9. Please merge ASAP. Thank you.
,
Oct 27 2017
This turned out to be depending on https://chromium-review.googlesource.com/c/chromium/src/+/703674, that is not mergable.
,
Oct 27 2017
Rejecting merge to M63 based on comment #11.
,
Nov 28 2017
Marking as fixed, since it's fixed on ToT.
,
Dec 8 2017
Issue 793031 has been merged into this issue.
,
Jan 3 2018
Issue 795336 has been merged into this issue. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by dgozman@chromium.org
, Oct 4 2017