Improve test coveage of WebProcessMemoryDumpImplTest

Improve test coveage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*

Committed: https://crrev.com/10345bedb439af0d8c17b3c6548383ca0da6aa64
Cr-Commit-Position: refs/heads/master@{#378423}

[hajimehoshi, 2016-02-26 08:00:30]

hajimehoshi@chromium.org changed reviewers:
+ bashi@chromium.org, haraken@chromium.org, primiano@chromium.org

[hajimehoshi, 2016-02-26 08:00:31]

primiano@: I just added minimum callers. Should I add checks for the return
values? As for createDiscardableMemoryAllocatorDump, as long as
TestDiscardableMemoryAllocator is used, we can't get a non-null return value.

Anyway, I think we should commit this CL after we make sure perf bots are green.

PTAL

[bashi, 2016-02-26 08:08:18]

Thanks for adding tests! I think we should wait for perf bots become green
before landing this.

https://codereview.chromium.org/1738463004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc
(right):

https://codereview.chromium.org/1738463004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc:133:
discardable_memory.get());
discardable_memory.unlock()

[haraken, 2016-02-26 08:42:50]

LGTM with bashi-san's comment addressed.

[hajimehoshi, 2016-02-29 07:00:47]

Thank you! I'd like to know primiano@'s opinion.

https://codereview.chromium.org/1738463004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc
(right):

https://codereview.chromium.org/1738463004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc:133:
discardable_memory.get());
On 2016/02/26 08:08:17, bashi1 wrote:
> discardable_memory.unlock()

Done.

[Primiano Tucci (use gerrit), 2016-02-29 16:15:29]

LGTM thanks

https://codereview.chromium.org/1738463004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc
(right):

https://codereview.chromium.org/1738463004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc:126:
ASSERT_NE(nullptr, skia_trace_memory_dump);
IIRC the ASSERT_NE incantation fails to expand properly this and will fail to
compile on some platforms (I hope I am wrong and it got fixed).
IIRC either you ASSERT_NE(static_cast<SkiaBlaBla*>(nullptr), ...) (or
equivalently cast the 2nd to void*)
or more simply ASSERT_TRUE(skia_trace_memory_dump)

[Primiano Tucci (use gerrit), 2016-02-29 16:16:24]

Ah, mind just typos in the CL title:
s/covertage/coverage/
s/WebProceesMemoryDumpImplTest/WebProcess.../

[hajimehoshi, 2016-03-01 09:40:24]

Description was changed from

==========
Improve test covertage of WebProceesMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

to

==========
Improve test covetage of WebProceesMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

[hajimehoshi, 2016-03-01 09:42:16]

Description was changed from

==========
Improve test covetage of WebProceesMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

to

==========
Improve test covetage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

[hajimehoshi, 2016-03-01 09:42:33]

Description was changed from

==========
Improve test covetage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

to

==========
Improve test coveage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

[hajimehoshi, 2016-03-01 10:06:23]

Thank you!

https://codereview.chromium.org/1738463004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc
(right):

https://codereview.chromium.org/1738463004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/web_process_memory_dump_impl_test.cc:126:
ASSERT_NE(nullptr, skia_trace_memory_dump);
On 2016/02/29 16:15:28, Primiano (throttled til Mar 4) wrote:
> IIRC the ASSERT_NE incantation fails to expand properly this and will fail to
> compile on some platforms (I hope I am wrong and it got fixed).
> IIRC either you ASSERT_NE(static_cast<SkiaBlaBla*>(nullptr), ...) (or
> equivalently cast the 2nd to void*)
> or more simply ASSERT_TRUE(skia_trace_memory_dump) 

Done. (Used ASSERT_TRUE)

[hajimehoshi, 2016-03-01 10:06:31]

The CQ bit was checked by hajimehoshi@chromium.org

[hajimehoshi, 2016-03-01 10:06:31]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/1738463004/#ps40001
(title: "Address Primiano's review")

[commit-bot: I haz the power, 2016-03-01 10:06:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1738463004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1738463004/40001

[commit-bot: I haz the power, 2016-03-01 11:07:54]

Description was changed from

==========
Improve test coveage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

to

==========
Improve test coveage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

[commit-bot: I haz the power, 2016-03-01 11:07:55]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-01 11:08:59]

Description was changed from

==========
Improve test coveage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*
==========

to

==========
Improve test coveage of WebProcessMemoryDumpImplTest

Before this CL, we missed to implement
createDiscardableMemoryAllocatorDump since methods including this were
not called in a unittest, and caused pref bots breakage (see also
crrev/1731893004).

This CL adds calls of such functions and improves test coverage.

BUG=548254
TEST=blink_platfrom_unittests --gtest_filter=WebProceesMemoryDumpImplTest.*

Committed: https://crrev.com/10345bedb439af0d8c17b3c6548383ca0da6aa64
Cr-Commit-Position: refs/heads/master@{#378423}
==========

[commit-bot: I haz the power, 2016-03-01 11:08:59]

Patchset 3 (id:??) landed as
https://crrev.com/10345bedb439af0d8c17b3c6548383ca0da6aa64
Cr-Commit-Position: refs/heads/master@{#378423}