Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(185)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 319543003: Create the managed_node at the ChromeBookmarkClient. (Closed)

Created:
6 years, 6 months ago by Joao da Silva
Modified:
6 years, 6 months ago
CC:
chromium-reviews, haitaol+watch_chromium.org, tfarina, maniscalco+watch_chromium.org, browser-components-watch_chromium.org, tim+watch_chromium.org
Visibility:
Public.

Description

Create the managed_node at the ChromeBookmarkClient. This change wires the ManagedBookmarksTracker to the ChromeBookmarkClient. The tracker keeps the new managed_node in sync with the ManagedBookmarks policy, and the ChromeBookmarkClient publishes that node as an extra node for the BookmarkModel. Also cleaned up some duplicated constants used by the managed bookmarks code. BUG=49598 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=275382

Patch Set 1 #

Patch Set 2 : added unittest #

Total comments: 4

Patch Set 3 : Fixed nits #

Patch Set 4 : Renamed check to IsDescendantOfManagedNode #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+500 lines, -111 lines) Patch
M chrome/browser/android/bookmarks/managed_bookmarks_shim.cc View 2 chunks +3 lines, -5 lines 0 comments Download
M chrome/browser/bookmarks/DEPS View 1 3 chunks +5 lines, -3 lines 0 comments Download
M chrome/browser/bookmarks/chrome_bookmark_client.h View 1 2 3 4 chunks +23 lines, -1 line 0 comments Download
M chrome/browser/bookmarks/chrome_bookmark_client.cc View 1 2 3 7 chunks +75 lines, -10 lines 0 comments Download
A chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc View 1 2 3 1 chunk +268 lines, -0 lines 0 comments Download
M chrome/browser/policy/managed_bookmarks_policy_handler.h View 1 chunk +0 lines, -4 lines 0 comments Download
M chrome/browser/policy/managed_bookmarks_policy_handler.cc View 3 chunks +7 lines, -10 lines 0 comments Download
M chrome/browser/sync/profile_sync_service_bookmark_unittest.cc View 1 2 3 6 chunks +24 lines, -4 lines 0 comments Download
M chrome/browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc View 1 4 chunks +36 lines, -32 lines 0 comments Download
M chrome/browser/ui/views/bookmarks/bookmark_editor_view.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M components/bookmarks.gypi View 1 2 chunks +3 lines, -0 lines 0 comments Download
A components/bookmarks/test/mock_bookmark_model_observer.h View 1 1 chunk +42 lines, -0 lines 0 comments Download
A components/bookmarks/test/mock_bookmark_model_observer.cc View 1 1 chunk +9 lines, -0 lines 0 comments Download
M components/policy/core/browser/managed_bookmarks_tracker_unittest.cc View 1 3 chunks +2 lines, -40 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
Joao da Silva
This has been split out of https://codereview.chromium.org/305973004 and should be hopefully easier to review :-) ...
6 years, 6 months ago (2014-06-04 22:52:55 UTC) #1
sky
bookmarks LGTM
6 years, 6 months ago (2014-06-05 15:01:59 UTC) #2
Joao da Silva
Added a unit test for the ChromeBookmarkClient in patch set 2.
6 years, 6 months ago (2014-06-05 15:40:56 UTC) #3
sky
SLGTM https://codereview.chromium.org/319543003/diff/20001/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc File chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc (right): https://codereview.chromium.org/319543003/diff/20001/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc#newcode134 chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc:134: }; private: DISALLOW... https://codereview.chromium.org/319543003/diff/20001/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc#newcode143 chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc:143: EXPECT_TRUE(client_->managed_node()->empty()); Maybe an: ...
6 years, 6 months ago (2014-06-05 16:03:56 UTC) #4
Joao da Silva
https://codereview.chromium.org/319543003/diff/20001/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc File chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc (right): https://codereview.chromium.org/319543003/diff/20001/chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc#newcode134 chrome/browser/bookmarks/chrome_bookmark_client_unittest.cc:134: }; On 2014/06/05 16:03:56, sky wrote: > private: DISALLOW... ...
6 years, 6 months ago (2014-06-05 16:57:12 UTC) #5
Joao da Silva
The CQ bit was checked by joaodasilva@chromium.org
6 years, 6 months ago (2014-06-05 23:35:04 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/319543003/80001
6 years, 6 months ago (2014-06-05 23:36:26 UTC) #7
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_triggered_tests on tryserver.chromium ...
6 years, 6 months ago (2014-06-06 05:06:19 UTC) #8
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-06 06:24:16 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered_tests/builds/160410)
6 years, 6 months ago (2014-06-06 06:24:17 UTC) #10
Joao da Silva
The CQ bit was checked by joaodasilva@chromium.org
6 years, 6 months ago (2014-06-06 08:35:24 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/319543003/80001
6 years, 6 months ago (2014-06-06 08:37:22 UTC) #12
commit-bot: I haz the power
6 years, 6 months ago (2014-06-06 10:44:43 UTC) #13
Message was sent while issue was closed. 
Change committed as 275382

Powered by Google App Engine
This is Rietveld 408576698