New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 851762 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 19
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

DevTools: only send the delta of the agent state from the renderer to the browser

Project Member Reported by pfeldman@chromium.org, Jun 12 2018

Issue description

Otherwise, script to evaluate on load could be heavy...
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bac15d70aa92de315d5b664d2978ad54c3931e1

commit 6bac15d70aa92de315d5b664d2978ad54c3931e1
Author: Johannes Henkel <johannes@chromium.org>
Date: Thu Aug 02 21:31:59 2018

Port InspectorIndexedDBAgent to InspectorAgentState fields.

There's just one field for the enabled bit so this one's trivial.

For reference the PR that introduced the new
InspectorSessionState / InspectorAgentState / Fields
was https://chromium-review.googlesource.com/c/chromium/src/+/1149201.
We're migrating things from state_ to this new mechanism,
which allows incremental updates to flow back to the browser
process.

Bug:  851762 
Change-Id: Ie709f8c99d196f5be018838e1cb9e70a2cb6aed8
Reviewed-on: https://chromium-review.googlesource.com/1159721
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580329}
[modify] https://crrev.com/6bac15d70aa92de315d5b664d2978ad54c3931e1/third_party/blink/renderer/modules/indexeddb/inspector_indexed_db_agent.cc
[modify] https://crrev.com/6bac15d70aa92de315d5b664d2978ad54c3931e1/third_party/blink/renderer/modules/indexeddb/inspector_indexed_db_agent.h

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dddb38e624f91fb13d33bdd6bb0f82015b9912ce

commit dddb38e624f91fb13d33bdd6bb0f82015b9912ce
Author: Johannes Henkel <johannes@chromium.org>
Date: Thu Aug 02 21:42:58 2018

Migrate emulation agent to InspectorSessionState fields...

... except for the virtual time properties. I'll do these
separately so it's possible to review them.

Bug:851762

Change-Id: I49333c7e0265cfc855cdb550e6f3338f74d1ebb1
Reviewed-on: https://chromium-review.googlesource.com/1157696
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580335}
[modify] https://crrev.com/dddb38e624f91fb13d33bdd6bb0f82015b9912ce/third_party/blink/renderer/core/inspector/inspector_emulation_agent.cc
[modify] https://crrev.com/dddb38e624f91fb13d33bdd6bb0f82015b9912ce/third_party/blink/renderer/core/inspector/inspector_emulation_agent.h

Changes I submitted earlier for addressing this bug:

Add InspectorAgentState for incremental state updates (Value).
https://chromium-review.googlesource.com/c/chromium/src/+/1149201

Migrate InspectorCSSAgent to the new InspectorSessionState.
https://chromium-review.googlesource.com/c/chromium/src/+/1153571

Migrate more agents to the new InspectorSessionState.
https://chromium-review.googlesource.com/c/chromium/src/+/1152695

Migrate DomDebuggerAgent to the new InspectorSessionState.
https://chromium-review.googlesource.com/c/chromium/src/+/1153296

Add bool InspectorAgentState::*Map::IsEmpty().
https://chromium-review.googlesource.com/c/chromium/src/+/1154038

Use IsEmpty for map keys.
https://chromium-review.googlesource.com/c/chromium/src/+/1154190

Use JSON to save the default background color override.
https://chromium-review.googlesource.com/c/chromium/src/+/1157248

Port InspectorLogAgent to use InspectorSessionState fields
https://chromium-review.googlesource.com/c/chromium/src/+/1157277

Port InspectorOverlayAgent to use InspectorAgentState fields.
https://chromium-review.googlesource.com/c/chromium/src/+/1159472

Introduce InspectorAgentState::Clear()
https://chromium-review.googlesource.com/c/chromium/src/+/1159822

Rename to agent_state_.ClearAllFields().
https://chromium-review.googlesource.com/c/chromium/src/+/1159996

Port InspectorPerformanceAgent to use an InspectorAgentState field.
https://chromium-review.googlesource.com/c/chromium/src/+/1159687

Port InspectorDatabaseAgent to InspectorAgentState fields.
https://chromium-review.googlesource.com/c/chromium/src/+/1159737

Port InspectorDOMStorageAgent to InspectorAgentState fields
https://chromium-review.googlesource.com/c/chromium/src/+/1159730
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/952567f39f93adb6b5c1d12c22643a29aa9e0a04

commit 952567f39f93adb6b5c1d12c22643a29aa9e0a04
Author: Johannes Henkel <johannes@chromium.org>
Date: Fri Aug 03 20:47:53 2018

Port virtual time to InspectorAgentState fields.

This is a refactoring except for
kVirtualTimeTaskStarvationCount. Previously, the code
called state_->setDouble(...VirtualTimeTaskStarvationCount, ...);
but then read it as an int, so it basically wouldn't get saved.
Now it's always an int.

Bug:  851762 
Change-Id: I288274ddddc44c910d89ef1fb8c1a6eacd54d8c5
Reviewed-on: https://chromium-review.googlesource.com/1161329
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580644}
[modify] https://crrev.com/952567f39f93adb6b5c1d12c22643a29aa9e0a04/third_party/blink/renderer/core/inspector/inspector_emulation_agent.cc
[modify] https://crrev.com/952567f39f93adb6b5c1d12c22643a29aa9e0a04/third_party/blink/renderer/core/inspector/inspector_emulation_agent.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d35e08b77ab808fb7539ca4ba58385dcb5f6f955

commit d35e08b77ab808fb7539ca4ba58385dcb5f6f955
Author: Johannes Henkel <johannes@chromium.org>
Date: Fri Aug 03 20:52:15 2018

Use ClearAllFields() when we're disabling.

inspector_network_agent.cc:
This is a behavior change, there are a fair number of fields here.
E.g., previously, the extra headers would be preserved between
disable() and enable(), now they're cleared out.

inspector_page_agent.cc:
Similarly, this is a behavior change.
E.g., previously, setFontFamilies and setFontSizes settings would
be preserved between disable() and enable(), now it's cleared out.

Bug:  851762 
Change-Id: I923a2d49ec92f395c5954116e115686517994a0f
Reviewed-on: https://chromium-review.googlesource.com/1161175
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580646}
[modify] https://crrev.com/d35e08b77ab808fb7539ca4ba58385dcb5f6f955/third_party/blink/renderer/core/inspector/inspector_network_agent.cc
[modify] https://crrev.com/d35e08b77ab808fb7539ca4ba58385dcb5f6f955/third_party/blink/renderer/core/inspector/inspector_page_agent.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85793c10bc6f75afbc7d44f614d06bbee018d552

commit 85793c10bc6f75afbc7d44f614d06bbee018d552
Author: Johannes Henkel <johannes@chromium.org>
Date: Fri Aug 03 22:25:09 2018

Port DeviceOrientationInspectorAgent to AgentStateFields.

I made it so that clearDeviceOrientationOverride() delegates
to disable(), which is now more careful about clearing out the state.

For reference the PR that introduced the new
InspectorSessionState / InspectorAgentState / Fields
was https://chromium-review.googlesource.com/c/chromium/src/+/1149201.
We're migrating things from state_ to this new mechanism,
which allows incremental updates to flow back to the browser
process.

Bug:851762

Change-Id: I388198b01da77e04943a4c23e90f521e25aeab94
Reviewed-on: https://chromium-review.googlesource.com/1159697
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580675}
[modify] https://crrev.com/85793c10bc6f75afbc7d44f614d06bbee018d552/third_party/blink/renderer/modules/device_orientation/device_orientation_inspector_agent.cc
[modify] https://crrev.com/85793c10bc6f75afbc7d44f614d06bbee018d552/third_party/blink/renderer/modules/device_orientation/device_orientation_inspector_agent.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41412080adef3c19f9d5ca86d25c277b522735e1

commit 41412080adef3c19f9d5ca86d25c277b522735e1
Author: Johannes Henkel <johannes@chromium.org>
Date: Fri Aug 03 22:28:10 2018

inspector_worker_agent: use ClearAllFields.

This is straight forward except for understanding what's up
with Restore. I've added a comment there, it may be
worth resolving in this change.

Bug:851762

Change-Id: I45b56b3f10dd96dae2b6c502030ced8e8ace026b
Reviewed-on: https://chromium-review.googlesource.com/1161178
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580677}
[modify] https://crrev.com/41412080adef3c19f9d5ca86d25c277b522735e1/third_party/blink/renderer/core/inspector/inspector_worker_agent.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f48cf76c2016e3dd04ea9be5dcea608971cb8e6f

commit f48cf76c2016e3dd04ea9be5dcea608971cb8e6f
Author: Johannes Henkel <johannes@chromium.org>
Date: Sat Aug 04 07:01:29 2018

Remove the InspectorAgentState::MapField::ClearAll method.

Use Clear() instead.
And for the dom_debugger_agent, use ClearAllFields.
This does reset the boolean field pauseOnAllXhrs as well,
  which is probably desired.

Bug:  851762 
Change-Id: I2a167892a37e1d8637ed4186ef90963a04d15753
Reviewed-on: https://chromium-review.googlesource.com/1162861
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580750}
[modify] https://crrev.com/f48cf76c2016e3dd04ea9be5dcea608971cb8e6f/third_party/blink/renderer/core/inspector/inspector_dom_debugger_agent.cc
[modify] https://crrev.com/f48cf76c2016e3dd04ea9be5dcea608971cb8e6f/third_party/blink/renderer/core/inspector/inspector_log_agent.cc
[modify] https://crrev.com/f48cf76c2016e3dd04ea9be5dcea608971cb8e6f/third_party/blink/renderer/core/inspector/inspector_session_state.h
[modify] https://crrev.com/f48cf76c2016e3dd04ea9be5dcea608971cb8e6f/third_party/blink/renderer/core/inspector/inspector_session_state_test.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ccf01bdba17a2d6364d48c5b5d1238854d3b2439

commit ccf01bdba17a2d6364d48c5b5d1238854d3b2439
Author: Johannes Henkel <johannes@chromium.org>
Date: Mon Aug 06 20:38:26 2018

Port InspectorDomAgent to InspectorAgentState fields.

InnerEnable does the actual work, but doesn't set the
flag, that only happens when enabled() is called (the
protocol method). It'd be ok to set it anyway, it doesn't
cause traffic on the wire, but I think it's a bit clearer
to protect against forgetting to set it with a DCHECK.
So this fixes the bug you pointed out when reviewing
https://chromium-review.googlesource.com/c/chromium/src/+/1162931
and keeps the change seperate.

Bug:  851762 
Change-Id: Ibd65a7e69539a760af6499be3f5ff96df6b3912b
Reviewed-on: https://chromium-review.googlesource.com/1163808
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580978}
[modify] https://crrev.com/ccf01bdba17a2d6364d48c5b5d1238854d3b2439/third_party/blink/renderer/core/inspector/inspector_dom_agent.cc
[modify] https://crrev.com/ccf01bdba17a2d6364d48c5b5d1238854d3b2439/third_party/blink/renderer/core/inspector/inspector_dom_agent.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3d2517b0ca04d1ae703aa9cf98142efcccecfa6

commit e3d2517b0ca04d1ae703aa9cf98142efcccecfa6
Author: Johannes Henkel <johannes@chromium.org>
Date: Mon Aug 06 23:12:03 2018

Port InspectorDOMSnapshotAgent to use InspectorAgentState.

There's just one enabled_ field here, a bool.

Bug:  851762 
Change-Id: I25247dc0d0333ea48ce2cf06c3fdf633ef2c1662
Reviewed-on: https://chromium-review.googlesource.com/1163998
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581033}
[modify] https://crrev.com/e3d2517b0ca04d1ae703aa9cf98142efcccecfa6/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.cc
[modify] https://crrev.com/e3d2517b0ca04d1ae703aa9cf98142efcccecfa6/third_party/blink/renderer/core/inspector/inspector_dom_snapshot_agent.h

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4731f799d9510da963fb1b3123c31a11d761eddb

commit 4731f799d9510da963fb1b3123c31a11d761eddb
Author: Johannes Henkel <johannes@chromium.org>
Date: Tue Aug 07 00:48:57 2018

Port InspectorApplicationCacheAgent to InspectorAgentState.

Bug:  851762 
Change-Id: I0208bb9a528c0e40cdb2b962461ac90225cb31b1
Reviewed-on: https://chromium-review.googlesource.com/1164062
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581073}
[modify] https://crrev.com/4731f799d9510da963fb1b3123c31a11d761eddb/third_party/blink/renderer/core/inspector/inspector_application_cache_agent.cc
[modify] https://crrev.com/4731f799d9510da963fb1b3123c31a11d761eddb/third_party/blink/renderer/core/inspector/inspector_application_cache_agent.h

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9

commit 281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9
Author: Johannes Henkel <johannes@chromium.org>
Date: Tue Aug 07 02:35:56 2018

Remove the old state cookie.

This removes the string parameters from the interfaces.
It also removes the json dictionaries that were representing
  the state.
v8 session continues to be encoded as JSON
  (see inspector_session.{h,cc}).

For reference, the cl that introduced the new way to manage
the agent state was this one (and advantage is that the updates
sent back to the browser process are deltas, not the entire
state):
https://chromium-review.googlesource.com/c/chromium/src/+/1149201
This change can get rid of the old way since all agents have
been migrated to the new way.

Bug:  851762 
Change-Id: Ie245c1c073eb006d4db19d819c2882e6a36f5b2b
Reviewed-on: https://chromium-review.googlesource.com/1162931
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581103}
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/content/browser/devtools/devtools_session.cc
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/content/browser/devtools/devtools_session.h
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/public/web/devtools_agent.mojom
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.h
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/inspector/inspector_base_agent.h
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/inspector/inspector_session.cc
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/inspector/inspector_session.h
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/inspector/worker_inspector_controller.cc
[modify] https://crrev.com/281ab38ea8f7abbbfafd0a04b0c3a2e9db6b06d9/third_party/blink/renderer/core/inspector/worker_inspector_controller.h

Owner: johannes@chromium.org
Status: Fixed (was: Assigned)

Sign in to add a comment