MD Bookmarks: Update a couple of TODOs

MD Bookmarks: Update a couple of TODOs

This completes one TODO (adding an error message to StoreClient),
and removes another obsolete TODO.

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

Review-Url: https://codereview.chromium.org/2858483002
Cr-Commit-Position: refs/heads/master@{#468574}
Committed: https://chromium.googlesource.com/chromium/src/+/2458d402ef27e41616c41e17e3e41d7bc683c196

[tsergeant, 2017-05-02 05:05:34]

Description was changed from

==========
MD Bookmarks: Update a couple of TODOs

This completes one TODO (adding an error message to StoreClient),
and removes another obsolete TODO.

BUG=697706
==========

to

==========
MD Bookmarks: Update a couple of TODOs

This completes one TODO (adding an error message to StoreClient),
and removes another obsolete TODO.

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

[tsergeant, 2017-05-02 05:09:09]

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

[tsergeant, 2017-05-02 05:09:11]

https://codereview.chromium.org/2858483002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/store_client.js (right):

https://codereview.chromium.org/2858483002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/store_client.js:52: if
(!this.getPropertyInfo(localProperty).defined) {
I originally TODO'd this because I thought it would be hard to deal with
behaviors...but this nifty function does it for us.

https://codereview.chromium.org/2858483002/diff/1/chrome/test/data/webui/md_b...
File chrome/test/data/webui/md_bookmarks/test_store.js (right):

https://codereview.chromium.org/2858483002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/test_store.js:59: * when changing a single
element).
It's been long enough to convince me that we don't need any sort of deep clone
behavior here: just making sure that tests update the right things is
sufficient.

[tsergeant, 2017-05-02 05:16:54]

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

[commit-bot: I haz the power, 2017-05-02 05:17:15]

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-05-02 05:25:27]

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

[commit-bot: I haz the power, 2017-05-02 05:25:28]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[calamity, 2017-05-02 05:35:59]

lgtm

https://codereview.chromium.org/2858483002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/store_client.js (right):

https://codereview.chromium.org/2858483002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/store_client.js:52: if
(!this.getPropertyInfo(localProperty).defined) {
On 2017/05/02 05:09:10, tsergeant wrote:
> I originally TODO'd this because I thought it would be hard to deal with
> behaviors...but this nifty function does it for us.

Heh.

Polymer 2.0:
Removed APIs
- element.getPropertyInfo: This api returned unexpected information some of the
time and was rarely used.

https://codereview.chromium.org/2858483002/diff/1/chrome/test/data/webui/md_b...
File chrome/test/data/webui/md_bookmarks/test_store.js (right):

https://codereview.chromium.org/2858483002/diff/1/chrome/test/data/webui/md_b...
chrome/test/data/webui/md_bookmarks/test_store.js:59: * when changing a single
element).
On 2017/05/02 05:09:10, tsergeant wrote:
> It's been long enough to convince me that we don't need any sort of deep clone
> behavior here: just making sure that tests update the right things is
> sufficient.

Agreed. I don't foresee any other major data members that will have these
problems.

[tsergeant, 2017-05-02 05:41:56]

https://codereview.chromium.org/2858483002/diff/1/chrome/browser/resources/md...
File chrome/browser/resources/md_bookmarks/store_client.js (right):

https://codereview.chromium.org/2858483002/diff/1/chrome/browser/resources/md...
chrome/browser/resources/md_bookmarks/store_client.js:52: if
(!this.getPropertyInfo(localProperty).defined) {
On 2017/05/02 05:35:59, calamity wrote:
> On 2017/05/02 05:09:10, tsergeant wrote:
> > I originally TODO'd this because I thought it would be hard to deal with
> > behaviors...but this nifty function does it for us.
> 
> Heh.
> 
> Polymer 2.0:
> Removed APIs
> - element.getPropertyInfo: This api returned unexpected information some of
the
> time and was rarely used.

😔

[tsergeant, 2017-05-02 05:47:50]

The CQ bit was checked by tsergeant@chromium.org

[commit-bot: I haz the power, 2017-05-02 05:48: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, 2017-05-02 06:49:51]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493704070933110, "parent_rev":
"dde5c7d288937174a16c1e5d6ca06e717b72e8d0", "commit_rev":
"2458d402ef27e41616c41e17e3e41d7bc683c196"}

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

Description was changed from

==========
MD Bookmarks: Update a couple of TODOs

This completes one TODO (adding an error message to StoreClient),
and removes another obsolete TODO.

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

to

==========
MD Bookmarks: Update a couple of TODOs

This completes one TODO (adding an error message to StoreClient),
and removes another obsolete TODO.

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

Review-Url: https://codereview.chromium.org/2858483002
Cr-Commit-Position: refs/heads/master@{#468574}
Committed:
https://chromium.googlesource.com/chromium/src/+/2458d402ef27e41616c41e17e3e4...
==========

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

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/2458d402ef27e41616c41e17e3e4...