[memory-infra] Add unittests for peak detection and NotifyDumpTriggered

[memory-infra] Add unittests for peak detection and NotifyDumpTriggered

BUG=607533

[ssid, 2017-03-11 00:00:00]

Patchset #1 (id:1) has been deleted

[ssid, 2017-03-11 00:02:53]

Patchset #1 (id:20001) has been deleted

[ssid, 2017-03-11 00:03:20]

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

[commit-bot: I haz the power, 2017-03-11 00:03:50]

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

[ssid, 2017-03-11 00:26:58]

ssid@chromium.org changed reviewers:
+ primiano@chromium.org

[ssid, 2017-03-11 00:26:58]

Sorry for doing this late. I just realized test coverage isn't enough.

[ssid, 2017-03-11 00:40:31]

On 2017/03/11 00:26:58, ssid wrote:
> Sorry for doing this late. I just realized test coverage isn't enough.

The rest of the methods are covered by MemoryDumpManagerTest(s) and cannot be
written without integration with MDM instance. Do you think I should move these
tests to memory_dump_manager_unittest as well?

[Primiano Tucci (use gerrit), 2017-03-16 19:05:04]

LGTM

>  Do you think I should move these
tests to memory_dump_manager_unittest as well?
Whatever you prefer really

[ssid, 2017-03-20 23:26:04]

ssid@chromium.org changed reviewers:
+ dcheng@chromium.org

[ssid, 2017-03-20 23:26:04]

+dcheng for one base/BUILD.gn file addition. ptal thanks!

[dcheng, 2017-03-21 06:48:01]

rs lgtm

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler_unittest.cc (right):

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler_unittest.cc:9: #include
"base/single_thread_task_runner.h"
Seems unused.

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler_unittest.cc:43: return
mds_->ShouldTriggerDump(total);
FWIW, this doesn't seem that useful as a helper (since mds_ itself is protected)

[ssid, 2017-03-22 00:24:19]

Thanks. comments inline.

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler_unittest.cc (right):

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler_unittest.cc:9: #include
"base/single_thread_task_runner.h"
On 2017/03/21 06:48:00, dcheng wrote:
> Seems unused.

Done.

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler_unittest.cc:43: return
mds_->ShouldTriggerDump(total);
On 2017/03/21 06:48:00, dcheng wrote:
> FWIW, this doesn't seem that useful as a helper (since mds_ itself is
protected)

MemoryDumpScheduler::ShouldTriggerDump is a private function and
MemoryDumpSchedulerPollingTest is the friend class. Didn't want to add each test
as friend.

[ssid, 2017-03-22 00:24:27]

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

[commit-bot: I haz the power, 2017-03-22 00:25:14]

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

[dcheng, 2017-03-22 00:28:10]

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler_unittest.cc (right):

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler_unittest.cc:43: return
mds_->ShouldTriggerDump(total);
On 2017/03/22 00:24:19, ssid wrote:
> On 2017/03/21 06:48:00, dcheng wrote:
> > FWIW, this doesn't seem that useful as a helper (since mds_ itself is
> protected)
> 
> MemoryDumpScheduler::ShouldTriggerDump is a private function and
> MemoryDumpSchedulerPollingTest is the friend class. Didn't want to add each
test
> as friend.

Oh duh. Sorry for missing that.

[commit-bot: I haz the power, 2017-03-22 00:34:06]

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

[commit-bot: I haz the power, 2017-03-22 00:34:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[ssid, 2017-03-22 00:38:42]

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler_unittest.cc (right):

https://codereview.chromium.org/2743663005/diff/60001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler_unittest.cc:9: #include
"base/single_thread_task_runner.h"
On 2017/03/22 00:24:19, ssid wrote:
> On 2017/03/21 06:48:00, dcheng wrote:
> > Seems unused.
> 
> Done.

Um, the include is needed since it's not included in header.

[ssid, 2017-03-22 00:38:47]

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

[commit-bot: I haz the power, 2017-03-22 00:39:28]

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

[ssid, 2017-03-22 01:22:59]

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

[commit-bot: I haz the power, 2017-03-22 01:23:45]

Dry run: 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, 2017-03-22 03:25:45]

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

[commit-bot: I haz the power, 2017-03-22 03:25:46]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[ssid, 2017-03-22 03:36:31]

The CQ bit was checked by ssid@chromium.org

[ssid, 2017-03-22 03:36:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2743663005/#ps120001
(title: "get back include")

[commit-bot: I haz the power, 2017-03-22 03:37:24]

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, 2017-03-22 05:31:06]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1490153791191420,
"parent_rev": "01ce65c1cff2ff093f1a3cd9d4cea6ca07d467a0", "commit_rev":
"e85f2a786db355e428813ba82a0533c3943ee552"}

[commit-bot: I haz the power, 2017-03-22 06:02:49]

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

[commit-bot: I haz the power, 2017-03-22 06:02:50]

Exceeded global retry quota

[Primiano Tucci (use gerrit), 2017-03-22 09:40:47]

> Exceeded global retry quota
Let's retry ticking commit again then. #AnarchyInTheUK

[Primiano Tucci (use gerrit), 2017-03-22 09:40:53]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2017-03-22 09:41:12]

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, 2017-03-22 09:46:05]

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

[commit-bot: I haz the power, 2017-03-22 09:46:06]

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

[ssid, 2017-03-22 13:29:32]

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

[commit-bot: I haz the power, 2017-03-22 13:29:49]

Dry run: 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, 2017-03-22 13:35:52]

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

[commit-bot: I haz the power, 2017-03-22 13:35:53]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[krasin1, 2017-03-22 15:59:59]

This CL broke CFI buildbots. Based on the fact that Rietveld shows a
chromium_presubmit bot failed and the issue is not marked as closed / submitted,
the CL was forced submitted from the command line. That makes it harder to
revert (I am preparing a revert right now).

What was the reason to bypass the presubmit queue? The CL does not look like an
emergency case.

[krasin1, 2017-03-22 16:00:55]

> What was the reason to bypass the presubmit queue? The CL does not look like
an
> emergency case.

Oh, I see. The bot "Exceeded global retry quota". Bad bot. Bad.

[Primiano Tucci (use gerrit), 2017-03-22 16:02:59]

On 2017/03/22 15:59:59, krasin1 wrote:
> This CL broke CFI buildbots. Based on the fact that Rietveld shows a
> chromium_presubmit bot failed and the issue is not marked as closed /
submitted,
> the CL was forced submitted from the command line. That makes it harder to
> revert (I am preparing a revert right now).
> 
> What was the reason to bypass the presubmit queue? The CL does not look like
an
> emergency case.

I definitely didn't force-land it (I just reticked commit). 
ssid, did you force land-it (I would be surprised considering that ssid is in
MTV and is barely morning there).

According to git log:
https://chromium.googlesource.com/chromium/src/+/e85f2a786db355e428813ba82a05...
committer:	Commit bot <commit-bot@chromium.org>

So looks like was committed by the CQ, even if the messages here don't show
that.
I have no idea what's happening. Will file an infra bug

[krasin1, 2017-03-22 16:05:40]

Revert created: https://codereview.chromium.org/2766933004/

[ssid, 2017-03-22 16:30:52]

On 2017/03/22 16:05:40, krasin1 wrote:
> Revert created: https://codereview.chromium.org/2766933004/

I didn't force submit. But I did click commit fuck multiple times and maybe
confused the CQ

[ssid, 2017-03-22 16:31:42]

On 2017/03/22 16:30:52, ssid wrote:
> On 2017/03/22 16:05:40, krasin1 wrote:
> > Revert created: https://codereview.chromium.org/2766933004/
> 
> I didn't force submit. But I did click commit  multiple times and maybe
> confused the CQ

Ouch autocorrect! I meant commit tick*!

[ssid, 2017-03-22 17:35:59]

closing since it landed.