[MD Bookmarks] Refactor window timer mocking.

[MD Bookmarks] Refactor window timer mocking.

This CL adds TimerProxy which allows elements to mock out setTimeout
and clearTimeout for testing purposes in a nicer way.

BUG=725786
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2926763005
Cr-Commit-Position: refs/heads/master@{#480354}
Committed: https://chromium.googlesource.com/chromium/src/+/12a52a35a4bfa982e7fb1c320d16c15691141957

[calamity, 2017-06-08 04:56:14]

Description was changed from

==========
[MD Bookmarks] Refactor window timer mocking.

This CL adds TimerProxy which allows elements to mock out setTimeout
and clearTimeout for testing purposes in a nicer way.

BUG=725786
==========

to

==========
[MD Bookmarks] Refactor window timer mocking.

This CL adds TimerProxy which allows elements to mock out setTimeout
and clearTimeout for testing purposes in a nicer way.

BUG=725786
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[calamity, 2017-06-08 07:50:15]

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

[commit-bot: I haz the power, 2017-06-08 07:50:26]

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-06-08 07:57:52]

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

[commit-bot: I haz the power, 2017-06-08 07:57:53]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[calamity, 2017-06-13 04:42:19]

Patchset #1 (id:1) has been deleted

[calamity, 2017-06-13 04:49:24]

Patchset #1 (id:20001) has been deleted

[calamity, 2017-06-13 05:04:36]

calamity@chromium.org changed reviewers:
+ tsergeant@chromium.org

[calamity, 2017-06-13 05:05:18]

Patchset #1 (id:40001) has been deleted

[tsergeant, 2017-06-13 06:48:43]

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/browser_...
File chrome/browser/browser_resources.grd (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/browser_...
chrome/browser/browser_resources.grd:299: <include
name="IDR_MD_BOOKMARKS_TIMER_PROXY_HTML"
file="resources\md_bookmarks\timer_proxy.html" type="BINDATA" />
Move this up, Timer sorts before Toast

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc:147:
source->AddResourcePath("timer_proxy.html",
As above, move this up

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_bookmarks/test_timer_proxy.js (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:14: /** @private
{Object<number, Function>} */
We can do better than Object:

!Map<number, !Function>

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:33: return
this.timeoutIds_++;
Optional: You could use cr.createUid() here, since it does basically exactly
what you're already doing.

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:55: * Returns whether a
given timeout id has been run or cleared.
This sentence is a little tough to read. How about

"Returns true if a given timeout id has not been run or cleared"

[calamity, 2017-06-15 05:11:38]

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

[commit-bot: I haz the power, 2017-06-15 05:11: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-06-15 06:21:55]

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

[commit-bot: I haz the power, 2017-06-15 06:21:56]

Dry run: This issue passed the CQ dry run.

[calamity, 2017-06-16 02:59:26]

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/browser_...
File chrome/browser/browser_resources.grd (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/browser_...
chrome/browser/browser_resources.grd:299: <include
name="IDR_MD_BOOKMARKS_TIMER_PROXY_HTML"
file="resources\md_bookmarks\timer_proxy.html" type="BINDATA" />
On 2017/06/13 06:48:43, tsergeant wrote:
> Move this up, Timer sorts before Toast

Done.

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/browser/ui/webui...
chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc:147:
source->AddResourcePath("timer_proxy.html",
On 2017/06/13 06:48:43, tsergeant wrote:
> As above, move this up

Done.

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_bookmarks/test_timer_proxy.js (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:14: /** @private
{Object<number, Function>} */
On 2017/06/13 06:48:43, tsergeant wrote:
> We can do better than Object:
> 
> !Map<number, !Function>

Done.

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:33: return
this.timeoutIds_++;
On 2017/06/13 06:48:43, tsergeant wrote:
> Optional: You could use cr.createUid() here, since it does basically exactly
> what you're already doing.

My ids start fighting with other uids which make me unable to test for exact
timeout ids in my test.

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:55: * Returns whether a
given timeout id has been run or cleared.
On 2017/06/13 06:48:43, tsergeant wrote:
> This sentence is a little tough to read. How about
> 
> "Returns true if a given timeout id has not been run or cleared"

Done.

[tsergeant, 2017-06-16 03:54:22]

lgtm

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
File chrome/test/data/webui/md_bookmarks/test_timer_proxy.js (right):

https://codereview.chromium.org/2926763005/diff/60001/chrome/test/data/webui/...
chrome/test/data/webui/md_bookmarks/test_timer_proxy.js:33: return
this.timeoutIds_++;
On 2017/06/16 02:59:26, calamity wrote:
> On 2017/06/13 06:48:43, tsergeant wrote:
> > Optional: You could use cr.createUid() here, since it does basically exactly
> > what you're already doing.
> 
> My ids start fighting with other uids which make me unable to test for exact
> timeout ids in my test.

Oh, sure, I forgot that you were hardcoding the values in the tests.

[calamity, 2017-06-19 03:56:11]

The CQ bit was checked by calamity@chromium.org

[commit-bot: I haz the power, 2017-06-19 03:56: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-06-19 05:30:09]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1497844571726540,
"parent_rev": "dda720249a41f271c31c85ab47cd64ed35a2b242", "commit_rev":
"12a52a35a4bfa982e7fb1c320d16c15691141957"}

[commit-bot: I haz the power, 2017-06-19 05:30:19]

Description was changed from

==========
[MD Bookmarks] Refactor window timer mocking.

This CL adds TimerProxy which allows elements to mock out setTimeout
and clearTimeout for testing purposes in a nicer way.

BUG=725786
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[MD Bookmarks] Refactor window timer mocking.

This CL adds TimerProxy which allows elements to mock out setTimeout
and clearTimeout for testing purposes in a nicer way.

BUG=725786
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2926763005
Cr-Commit-Position: refs/heads/master@{#480354}
Committed:
https://chromium.googlesource.com/chromium/src/+/12a52a35a4bfa982e7fb1c320d16...
==========

[commit-bot: I haz the power, 2017-06-19 05:30:20]

Committed patchset #3 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/12a52a35a4bfa982e7fb1c320d16...