Revert of [Mac] Refactor bookmark bar controller

Revert of [Mac] Refactor bookmark bar controller (patchset #10 id:180001 of https://codereview.chromium.org/2751573002/ )

Reason for revert:
Reverting as likely caused msan failurs. See https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FMac_ASan_64_Tests__1_%2F29660%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Flogs%2FBookmarkFolderAppleScriptTest.DeleteBookmarkItems%2F0 for example:

==77706==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000445b40 at pc 0x0001156ab4fe bp 0x7fff5c399fd0 sp 0x7fff5c399fc8
READ of size 8 at 0x603000445b40 thread T0
    #0 0x1156ab4fd in -[BookmarkBarController applyLayout:animated:] ??:0:0
    #1 0x1156a9db1 in -[BookmarkBarController rebuildLayoutWithAnimated:] ??:0:0
    #2 0x1156acf25 in -[BookmarkBarController nodeRemoved:parent:index:] ??:0:0
    #3 0x1127774c7 in bookmarks::BookmarkModel::RemoveAndDeleteNode(bookmarks::BookmarkNode*) ??:0:0
    #4 0x112776d40 in bookmarks::BookmarkModel::Remove(bookmarks::BookmarkNode const*) ??:0:0
    #5 0x10607a64c in (anonymous namespace)::BookmarkFolderAppleScriptTest_DeleteBookmarkItems_Test::RunTestOnMainThread() ??:0:0
    #6 0x10d1f6ac3 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() ??:0:0
    #7 0x10bb87dc3 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ??:0:0
    #8 0x10bb851e6 in ChromeBrowserMainParts::PreMainMessageLoopRun() ??:0:0
    #9 0x1077b02dd in content::BrowserMainLoop::PreMainMessageLoopRun() ??:0:0
    #10 0x1084ba8a3 in content::StartupTaskRunner::RunAllTasksNow() ??:0:0
    #11 0x1077abc3a in content::BrowserMainLoop::CreateStartupTasks() ??:0:0
    #12 0x1077b94ec in content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&) ??:0:0
    #13 0x1077a40b5 in content::BrowserMain(content::MainFunctionParams const&) ??:0:0
    #14 0x10b6a9604 in content::RunNamedProcessTypeMain(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, content::MainFunctionParams const&, content::ContentMainDelegate*) ??:0:0
    #15 0x10b6ab3c7 in content::ContentMainRunnerImpl::Run() ??:0:0
    #16 0x111afc88b in service_manager::Main(service_manager::MainParams const&) ??:0:0
    #17 0x10b6a8ff4 in content::ContentMain(content::ContentMainParams const&) ??:0:0
    #18 0x10d1f5c4f in content::BrowserTestBase::SetUp() ??:0:0
    #19 0x10ba09201 in InProcessBrowserTest::SetUp() ??:0:0
    #20 0x10ed76b2f in testing::Test::Run() ??:0:0
    #21 0x10ed78b43 in testing::TestInfo::Run() ??:0:0
    #22 0x10ed79e86 in testing::TestCase::Run() ??:0:0
    #23 0x10ed8cfb6 in testing::internal::UnitTestImpl::RunAllTests() ??:0:0
    #24 0x10ed8c588 in testing::UnitTest::Run() ??:0:0
    #25 0x10ba51d3e in base::TestSuite::Run() ??:0:0
    #26 0x10b6fc3ac in ChromeTestSuiteRunner::RunTestSuite(int, char**) ??:0:0
    #27 0x10d2c7991 in content::LaunchTests(content::TestLauncherDelegate*, int, int, char**) ??:0:0
    #28 0x10b6fc203 in main ??:0:0
    #29 0x7fff8a0125fc in start ??:0:0


Original issue's description:
> Yes, with RTL thrown in, since this is specifically designed to make it almost free.
> 
> Two major things here:
> - The bar is no longer relaid-out directly in response to changes in view size, bookmark model etc. Instead, a new UI-direction-agnostic view model (BookmarkBarLayout) is created from the current state, and if it's different from before, it's applied to the view.
> - Removed a bunch of layout-related code that's no longer necessary
> 
> BUG=648560
> 
> Review-Url: https://codereview.chromium.org/2751573002
> Cr-Commit-Position: refs/heads/master@{#468364}
> Committed: https://chromium.googlesource.com/chromium/src/+/2644729cb7722a702a76cc2758d0ce372e1e6f92

TBR=ellyjones@chromium.org,avi@chromium.org,lgrey@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=648560
Review-Url: https://codereview.chromium.org/2853123002
Cr-Commit-Position: refs/heads/master@{#468398}
Committed: https://chromium.googlesource.com/chromium/src/+/27ed7650a26ddb0f447963a0c6b5ff1e9ed0e994

[sky, 2017-05-01 20:27:04]

The CQ bit was checked by sky@chromium.org

[sky, 2017-05-01 20:27:05]

Created Revert of [Mac] Refactor bookmark bar controller

[commit-bot: I haz the power, 2017-05-01 20:28:00]

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-01 20:28:33]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493670424383200, "parent_rev":
"6c3d410ea272423ac277dc3f3e3def6490dfd6dc", "commit_rev":
"27ed7650a26ddb0f447963a0c6b5ff1e9ed0e994"}

[commit-bot: I haz the power, 2017-05-01 20:29:04]

Description was changed from

==========
Revert of [Mac] Refactor bookmark bar controller (patchset #10 id:180001 of
https://codereview.chromium.org/2751573002/ )

Reason for revert:
Reverting as likely caused msan failurs. See
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FMac_AS...
for example:

==77706==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000445b40
at pc 0x0001156ab4fe bp 0x7fff5c399fd0 sp 0x7fff5c399fc8
READ of size 8 at 0x603000445b40 thread T0
    #0 0x1156ab4fd in -[BookmarkBarController applyLayout:animated:] ??:0:0
    #1 0x1156a9db1 in -[BookmarkBarController rebuildLayoutWithAnimated:] ??:0:0
    #2 0x1156acf25 in -[BookmarkBarController nodeRemoved:parent:index:] ??:0:0
    #3 0x1127774c7 in
bookmarks::BookmarkModel::RemoveAndDeleteNode(bookmarks::BookmarkNode*) ??:0:0
    #4 0x112776d40 in bookmarks::BookmarkModel::Remove(bookmarks::BookmarkNode
const*) ??:0:0
    #5 0x10607a64c in (anonymous
namespace)::BookmarkFolderAppleScriptTest_DeleteBookmarkItems_Test::RunTestOnMainThread()
??:0:0
    #6 0x10d1f6ac3 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
??:0:0
    #7 0x10bb87dc3 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ??:0:0
    #8 0x10bb851e6 in ChromeBrowserMainParts::PreMainMessageLoopRun() ??:0:0
    #9 0x1077b02dd in content::BrowserMainLoop::PreMainMessageLoopRun() ??:0:0
    #10 0x1084ba8a3 in content::StartupTaskRunner::RunAllTasksNow() ??:0:0
    #11 0x1077abc3a in content::BrowserMainLoop::CreateStartupTasks() ??:0:0
    #12 0x1077b94ec in
content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&)
??:0:0
    #13 0x1077a40b5 in content::BrowserMain(content::MainFunctionParams const&)
??:0:0
    #14 0x10b6a9604 in
content::RunNamedProcessTypeMain(std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > const&,
content::MainFunctionParams const&, content::ContentMainDelegate*) ??:0:0
    #15 0x10b6ab3c7 in content::ContentMainRunnerImpl::Run() ??:0:0
    #16 0x111afc88b in service_manager::Main(service_manager::MainParams const&)
??:0:0
    #17 0x10b6a8ff4 in content::ContentMain(content::ContentMainParams const&)
??:0:0
    #18 0x10d1f5c4f in content::BrowserTestBase::SetUp() ??:0:0
    #19 0x10ba09201 in InProcessBrowserTest::SetUp() ??:0:0
    #20 0x10ed76b2f in testing::Test::Run() ??:0:0
    #21 0x10ed78b43 in testing::TestInfo::Run() ??:0:0
    #22 0x10ed79e86 in testing::TestCase::Run() ??:0:0
    #23 0x10ed8cfb6 in testing::internal::UnitTestImpl::RunAllTests() ??:0:0
    #24 0x10ed8c588 in testing::UnitTest::Run() ??:0:0
    #25 0x10ba51d3e in base::TestSuite::Run() ??:0:0
    #26 0x10b6fc3ac in ChromeTestSuiteRunner::RunTestSuite(int, char**) ??:0:0
    #27 0x10d2c7991 in content::LaunchTests(content::TestLauncherDelegate*, int,
int, char**) ??:0:0
    #28 0x10b6fc203 in main ??:0:0
    #29 0x7fff8a0125fc in start ??:0:0


Original issue's description:
> Yes, with RTL thrown in, since this is specifically designed to make it almost
free.
> 
> Two major things here:
> - The bar is no longer relaid-out directly in response to changes in view
size, bookmark model etc. Instead, a new UI-direction-agnostic view model
(BookmarkBarLayout) is created from the current state, and if it's different
from before, it's applied to the view.
> - Removed a bunch of layout-related code that's no longer necessary
> 
> BUG=648560
> 
> Review-Url: https://codereview.chromium.org/2751573002
> Cr-Commit-Position: refs/heads/master@{#468364}
> Committed:
https://chromium.googlesource.com/chromium/src/+/2644729cb7722a702a76cc2758d0...

TBR=ellyjones@chromium.org,avi@chromium.org,lgrey@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=648560
==========

to

==========
Revert of [Mac] Refactor bookmark bar controller (patchset #10 id:180001 of
https://codereview.chromium.org/2751573002/ )

Reason for revert:
Reverting as likely caused msan failurs. See
https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.memory%2FMac_AS...
for example:

==77706==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000445b40
at pc 0x0001156ab4fe bp 0x7fff5c399fd0 sp 0x7fff5c399fc8
READ of size 8 at 0x603000445b40 thread T0
    #0 0x1156ab4fd in -[BookmarkBarController applyLayout:animated:] ??:0:0
    #1 0x1156a9db1 in -[BookmarkBarController rebuildLayoutWithAnimated:] ??:0:0
    #2 0x1156acf25 in -[BookmarkBarController nodeRemoved:parent:index:] ??:0:0
    #3 0x1127774c7 in
bookmarks::BookmarkModel::RemoveAndDeleteNode(bookmarks::BookmarkNode*) ??:0:0
    #4 0x112776d40 in bookmarks::BookmarkModel::Remove(bookmarks::BookmarkNode
const*) ??:0:0
    #5 0x10607a64c in (anonymous
namespace)::BookmarkFolderAppleScriptTest_DeleteBookmarkItems_Test::RunTestOnMainThread()
??:0:0
    #6 0x10d1f6ac3 in content::BrowserTestBase::ProxyRunTestOnMainThreadLoop()
??:0:0
    #7 0x10bb87dc3 in ChromeBrowserMainParts::PreMainMessageLoopRunImpl() ??:0:0
    #8 0x10bb851e6 in ChromeBrowserMainParts::PreMainMessageLoopRun() ??:0:0
    #9 0x1077b02dd in content::BrowserMainLoop::PreMainMessageLoopRun() ??:0:0
    #10 0x1084ba8a3 in content::StartupTaskRunner::RunAllTasksNow() ??:0:0
    #11 0x1077abc3a in content::BrowserMainLoop::CreateStartupTasks() ??:0:0
    #12 0x1077b94ec in
content::BrowserMainRunnerImpl::Initialize(content::MainFunctionParams const&)
??:0:0
    #13 0x1077a40b5 in content::BrowserMain(content::MainFunctionParams const&)
??:0:0
    #14 0x10b6a9604 in
content::RunNamedProcessTypeMain(std::__1::basic_string<char,
std::__1::char_traits<char>, std::__1::allocator<char> > const&,
content::MainFunctionParams const&, content::ContentMainDelegate*) ??:0:0
    #15 0x10b6ab3c7 in content::ContentMainRunnerImpl::Run() ??:0:0
    #16 0x111afc88b in service_manager::Main(service_manager::MainParams const&)
??:0:0
    #17 0x10b6a8ff4 in content::ContentMain(content::ContentMainParams const&)
??:0:0
    #18 0x10d1f5c4f in content::BrowserTestBase::SetUp() ??:0:0
    #19 0x10ba09201 in InProcessBrowserTest::SetUp() ??:0:0
    #20 0x10ed76b2f in testing::Test::Run() ??:0:0
    #21 0x10ed78b43 in testing::TestInfo::Run() ??:0:0
    #22 0x10ed79e86 in testing::TestCase::Run() ??:0:0
    #23 0x10ed8cfb6 in testing::internal::UnitTestImpl::RunAllTests() ??:0:0
    #24 0x10ed8c588 in testing::UnitTest::Run() ??:0:0
    #25 0x10ba51d3e in base::TestSuite::Run() ??:0:0
    #26 0x10b6fc3ac in ChromeTestSuiteRunner::RunTestSuite(int, char**) ??:0:0
    #27 0x10d2c7991 in content::LaunchTests(content::TestLauncherDelegate*, int,
int, char**) ??:0:0
    #28 0x10b6fc203 in main ??:0:0
    #29 0x7fff8a0125fc in start ??:0:0


Original issue's description:
> Yes, with RTL thrown in, since this is specifically designed to make it almost
free.
> 
> Two major things here:
> - The bar is no longer relaid-out directly in response to changes in view
size, bookmark model etc. Instead, a new UI-direction-agnostic view model
(BookmarkBarLayout) is created from the current state, and if it's different
from before, it's applied to the view.
> - Removed a bunch of layout-related code that's no longer necessary
> 
> BUG=648560
> 
> Review-Url: https://codereview.chromium.org/2751573002
> Cr-Commit-Position: refs/heads/master@{#468364}
> Committed:
https://chromium.googlesource.com/chromium/src/+/2644729cb7722a702a76cc2758d0...

TBR=ellyjones@chromium.org,avi@chromium.org,lgrey@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=648560

Review-Url: https://codereview.chromium.org/2853123002
Cr-Commit-Position: refs/heads/master@{#468398}
Committed:
https://chromium.googlesource.com/chromium/src/+/27ed7650a26ddb0f447963a0c6b5...
==========

[commit-bot: I haz the power, 2017-05-01 20:29:05]

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