Updating modification time of parent when a folder node is added in it in bookmark bar.

Updating modification time of parent when a folder node is added in it in bookmark bar.

Modification time of the parent node was not getting updated when a folder node
is getting added in it in bookmark bar, but parent node modification time is getting
updated after adding a node in it via boookmark manager.
Nodes appears in the order of latest modification time at top.

BUG=175707
Committed: https://crrev.com/347a2817bfcdf3190b92cc24d6b473dc93ce8b3d
Cr-Commit-Position: refs/heads/master@{#328056}

[Deepak, 2015-04-23 13:24:59]

deepak.m1@samsung.com changed reviewers:
+ rockot@chromium.org

[Deepak, 2015-04-23 13:25:00]

Please review.
Thanks

[Deepak, 2015-04-27 14:15:27]

Please review.
Thanks

[Ken Rockot(use gerrit already), 2015-04-28 03:14:39]

Sorry for the delay. Comment inline.

https://codereview.chromium.org/1101833002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/bookmarks/bookmarks_api.cc (right):

https://codereview.chromium.org/1101833002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:193:
model->SetDateFolderModified(parent, base::Time::Now());
This looks like the wrong code to change. Shouldn't you instead add a call to
parent->set_date_folder_modified from within
BookmarkModel::AddFolderWithMetaInfo?

On an tangential note, the if-block after the DCHECK is effectively unreachable
and should probably be removed.

[Deepak, 2015-04-28 11:23:33]

PTAL

https://codereview.chromium.org/1101833002/diff/1/chrome/browser/extensions/a...
File chrome/browser/extensions/api/bookmarks/bookmarks_api.cc (right):

https://codereview.chromium.org/1101833002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:193:
model->SetDateFolderModified(parent, base::Time::Now());
On 2015/04/28 03:14:39, Ken Rockot wrote:
> This looks like the wrong code to change. Shouldn't you instead add a call to
> parent->set_date_folder_modified from within
> BookmarkModel::AddFolderWithMetaInfo?
> 
> On an tangential note, the if-block after the DCHECK is effectively
unreachable
> and should probably be removed.

I have done changes due to 2 reasons:
1) I have followed the way parent node is getting modified when we add folders
via bookmark manager.
It is adding the folder node and then modifying the parent modification time in
BookmarkNode* CreateNewNode().

2)we should not modify parent folder time till new node get added in it. and
AddNode() called last in BookmarkModel::AddFolderWithMetaInfo().

Bookmark model is accessed by both bookmark bar as well as bookmark manager, so
does not wanted to disturb functionality here.

I agree with you that if condition is not reachable as DCHECK() is their.
I am removing this code and putting by code in the else case.

Please correct me if I misunderstood something.
Thanks

[Deepak, 2015-04-30 12:21:40]

Please share your opinion about #5 comment.
Thanks

[Ken Rockot(use gerrit already), 2015-04-30 20:57:05]

Ah OK. I agree with your reasoning. :)

LGTM, and thanks!

[Deepak, 2015-05-02 04:33:19]

The CQ bit was checked by deepak.m1@samsung.com

[commit-bot: I haz the power, 2015-05-02 04:33:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1101833002/20001

[commit-bot: I haz the power, 2015-05-02 06:51:12]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-02 06:52:03]

Patchset 2 (id:??) landed as
https://crrev.com/347a2817bfcdf3190b92cc24d6b473dc93ce8b3d
Cr-Commit-Position: refs/heads/master@{#328056}