[DevTools] Allow modifying instrumenting agents while iterating.

[DevTools] Allow modifying instrumenting agents while iterating.

In this case we stop iteration early (possibly loosing instrumentation call though).

This can happen if during instrumentation call we paused the execution, entered
nested message loop and detached from there (thus removing agent from the list).

BUG=none

Committed: https://crrev.com/a0206e7e8481b0bd171beb9fc895bd55201eff6e
Cr-Commit-Position: refs/heads/master@{#404694}

[dgozman, 2016-07-08 23:43:57]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[dgozman, 2016-07-08 23:43:58]

Take a look please.

[dgozman, 2016-07-08 23:44:03]

The CQ bit was checked by dgozman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-08 23:44:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[caseq, 2016-07-09 00:37:52]

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py
(right):

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:107:
class CORE_EXPORT AgentList {
drop CORE_EXPORT

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:110:
class CORE_EXPORT Iterator {
ditto

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:119:
private:
nit: blank line above

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:123:
Member<T>* m_value;
Why not HeapVector<Member<T>>::iterator?

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:176:
++${member_name}.m_version;
Let's move this logic to to AgentList::remove()?

[commit-bot: I haz the power, 2016-07-09 00:39:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-09 00:39:21]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[dgozman, 2016-07-09 00:50:41]

PTAL

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py
(right):

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:107:
class CORE_EXPORT AgentList {
On 2016/07/09 00:37:52, caseq wrote:
> drop CORE_EXPORT

Done.

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:110:
class CORE_EXPORT Iterator {
On 2016/07/09 00:37:52, caseq wrote:
> ditto

Done.

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:119:
private:
On 2016/07/09 00:37:51, caseq wrote:
> nit: blank line above

Done.

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:123:
Member<T>* m_value;
On 2016/07/09 00:37:51, caseq wrote:
> Why not HeapVector<Member<T>>::iterator?

Done.

https://codereview.chromium.org/2139443002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:176:
++${member_name}.m_version;
On 2016/07/09 00:37:52, caseq wrote:
> Let's move this logic to to AgentList::remove()?

Done.

[dgozman, 2016-07-09 00:50:49]

The CQ bit was checked by dgozman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-09 00:51:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[caseq, 2016-07-09 01:18:34]

lgtm

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py
(right):

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:119:
bool operator==(const Iterator& other) { return isEnd() && other.isEnd() ? true
: m_value == other.m_value; }
make const? Also, consider isEnd() && other.isEnd() || m_value == other.m_value

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:120:
bool operator!=(const Iterator& other) { return !(*this == other); }
const

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:128:
InnerIterator m_value;
empty line between methods and data please!

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:144:
AgentsList() : m_version(0) { }
here and below, empty lines between all methods please!

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:148:
for (size_t i = 0; i < m_vector.size(); ++i) {
just
if (m_vector.contains(value))
    return;

[dgozman, 2016-07-09 02:00:21]

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py
(right):

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:119:
bool operator==(const Iterator& other) { return isEnd() && other.isEnd() ? true
: m_value == other.m_value; }
On 2016/07/09 01:18:34, caseq wrote:
> make const? Also, consider isEnd() && other.isEnd() || m_value ==
other.m_value

Done.

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:128:
InnerIterator m_value;
On 2016/07/09 01:18:34, caseq wrote:
> empty line between methods and data please!

Done.

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:144:
AgentsList() : m_version(0) { }
On 2016/07/09 01:18:34, caseq wrote:
> here and below, empty lines between all methods please!

Done.

https://codereview.chromium.org/2139443002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/inspector/CodeGeneratorInstrumentation.py:148:
for (size_t i = 0; i < m_vector.size(); ++i) {
On 2016/07/09 01:18:34, caseq wrote:
> just
> if (m_vector.contains(value))
>     return;

Done.

[dgozman, 2016-07-09 02:00:25]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-07-09 02:00:26]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/2139443002/#ps40001
(title: "more comments addressed")

[commit-bot: I haz the power, 2016-07-09 02:00:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-09 04:01:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-09 04:01:28]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[dgozman, 2016-07-09 04:35:17]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-07-09 04:35:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-09 06:36:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-09 06:36:04]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[dgozman, 2016-07-09 13:59:27]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-07-09 13:59:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-09 16:00:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-09 16:00:11]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[dgozman, 2016-07-09 18:13:53]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-07-09 18:14:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-09 19:37:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-09 19:37:52]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dgozman, 2016-07-09 20:02:45]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-07-09 20:02:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-09 22:54:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-09 22:54:26]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dgozman, 2016-07-09 22:59:00]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-07-09 22:59:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-10 01:53:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-10 01:53:33]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dgozman, 2016-07-10 01:57:28]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2016-07-10 01:57:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-10 04:51:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-10 04:51:52]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dgozman, 2016-07-11 17:18:48]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2016-07-11 17:18:49]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org
Link to the patchset: https://codereview.chromium.org/2139443002/#ps60001
(title: "rebased")

[commit-bot: I haz the power, 2016-07-11 17:19:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-11 18:30:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-11 18:31:04]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-11 18:35:02]

Description was changed from

==========
[DevTools] Allow modifying instrumenting agents while iterating.

In this case we stop iteration early (possibly loosing instrumentation call
though).

This can happen if during instrumentation call we paused the execution, entered
nested message loop and detached from there (thus removing agent from the list).

BUG=none
==========

to

==========
[DevTools] Allow modifying instrumenting agents while iterating.

In this case we stop iteration early (possibly loosing instrumentation call
though).

This can happen if during instrumentation call we paused the execution, entered
nested message loop and detached from there (thus removing agent from the list).

BUG=none

Committed: https://crrev.com/a0206e7e8481b0bd171beb9fc895bd55201eff6e
Cr-Commit-Position: refs/heads/master@{#404694}
==========

[commit-bot: I haz the power, 2016-07-11 18:35:03]

Patchset 4 (id:??) landed as
https://crrev.com/a0206e7e8481b0bd171beb9fc895bd55201eff6e
Cr-Commit-Position: refs/heads/master@{#404694}

[dgozman, 2016-07-12 20:43:29]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2143053004/ by dgozman@chromium.org.

The reason for reverting is:
https://bugs.chromium.org/p/chromium/issues/detail?id=627577.

[pfeldman, 2016-07-12 22:53:13]

Do we need that much code to fix it?