Save and restore unified mode

Save and restore unified mode

BUG=489809
TEST=DisplayPreferenceTest.SaveUnifiedMode/RestoreUnifiedMode

Committed: https://crrev.com/b83c400b39840d0c18bd113559ea0040eb3c6b46
Cr-Commit-Position: refs/heads/master@{#331076}

Committed: https://crrev.com/bba2d991127c5cdb841e88df1c5613045cabc23c
Cr-Commit-Position: refs/heads/master@{#331149}

[oshima, 2015-05-21 09:15:23]

Patchset #1 (id:1) has been deleted

[oshima, 2015-05-21 09:15:30]

Patchset #1 (id:20001) has been deleted

[oshima, 2015-05-21 09:20:06]

Patchset #1 (id:40001) has been deleted

[oshima, 2015-05-21 09:20:59]

oshima@chromium.org changed reviewers:
+ mukai@chromium.org

[oshima, 2015-05-21 09:21:21]

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

[commit-bot: I haz the power, 2015-05-21 09:21:43]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1150913002/60001

[Jun Mukai, 2015-05-21 09:38:12]

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_lay...
File ash/display/display_layout.h (right):

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_lay...
ash/display/display_layout.h:63: bool default_unified;
Could this be merged into mirrored mode and make it as an enum (like Position
value)?

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_man...
File ash/display/display_manager.cc (right):

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_man...
ash/display/display_manager.cc:657: default_multi_display_mode_ =
layout.default_unified ? UNIFIED : EXTENDED;
(layout.default_unified && ash::switches::UnifiedDesktopEnabled()) ?
It can't exit from the unified mode if the user turns off the flag.

[commit-bot: I haz the power, 2015-05-21 10:06:41]

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

[commit-bot: I haz the power, 2015-05-21 10:06:42]

Dry run: Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[oshima, 2015-05-21 12:24:12]

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_lay...
File ash/display/display_layout.h (right):

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_lay...
ash/display/display_layout.h:63: bool default_unified;
On 2015/05/21 09:38:12, Jun Mukai wrote:
> Could this be merged into mirrored mode and make it as an enum (like Position
> value)?

I need this separated so that we can switch back to correct mode when unmirror.

We can revisit the operation (cycle through modes) in 45, but I want to keep
this as is.

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_man...
File ash/display/display_manager.cc (right):

https://codereview.chromium.org/1150913002/diff/60001/ash/display/display_man...
ash/display/display_manager.cc:657: default_multi_display_mode_ =
layout.default_unified ? UNIFIED : EXTENDED;
On 2015/05/21 09:38:12, Jun Mukai wrote:
> (layout.default_unified && ash::switches::UnifiedDesktopEnabled()) ?
> It can't exit from the unified mode if the user turns off the flag.

good point. Done. Also updated tests because tests now require the flag.

[oshima, 2015-05-21 12:24:21]

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

[commit-bot: I haz the power, 2015-05-21 12:24:26]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1150913002/80001

[commit-bot: I haz the power, 2015-05-21 12:49:45]

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

[commit-bot: I haz the power, 2015-05-21 12:49:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jun Mukai, 2015-05-21 13:14:20]

lgtm

https://codereview.chromium.org/1150913002/diff/80001/ash/display/display_lay...
File ash/display/display_layout_store.cc (right):

https://codereview.chromium.org/1150913002/diff/80001/ash/display/display_lay...
ash/display/display_layout_store.cc:75: LOG(ERROR) << "default unified:" <<
default_unified;
Remove this log

[oshima, 2015-05-21 16:34:52]

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

[oshima, 2015-05-21 16:34:52]

The patchset sent to the CQ was uploaded after l-g-t-m from mukai@chromium.org
Link to the patchset: https://codereview.chromium.org/1150913002/#ps100001
(title: " ")

[oshima, 2015-05-21 16:35:04]

https://codereview.chromium.org/1150913002/diff/80001/ash/display/display_lay...
File ash/display/display_layout_store.cc (right):

https://codereview.chromium.org/1150913002/diff/80001/ash/display/display_lay...
ash/display/display_layout_store.cc:75: LOG(ERROR) << "default unified:" <<
default_unified;
On 2015/05/21 13:14:19, Jun Mukai wrote:
> Remove this log

Done.

[commit-bot: I haz the power, 2015-05-21 16:35:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1150913002/100001

[commit-bot: I haz the power, 2015-05-21 16:53:32]

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

[commit-bot: I haz the power, 2015-05-21 16:53:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_gn_dbg on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[oshima, 2015-05-22 07:01:30]

The CQ bit was checked by oshima@chromium.org

[oshima, 2015-05-22 07:01:31]

The patchset sent to the CQ was uploaded after l-g-t-m from mukai@chromium.org
Link to the patchset: https://codereview.chromium.org/1150913002/#ps140001
(title: " ")

[commit-bot: I haz the power, 2015-05-22 07:01:54]

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

[commit-bot: I haz the power, 2015-05-22 07:07:13]

Committed patchset #5 (id:140001)

[commit-bot: I haz the power, 2015-05-22 07:08:03]

Patchset 5 (id:??) landed as
https://crrev.com/b83c400b39840d0c18bd113559ea0040eb3c6b46
Cr-Commit-Position: refs/heads/master@{#331076}

[battre, 2015-05-22 09:45:32]

battre@chromium.org changed reviewers:
+ battre@chromium.org

[battre, 2015-05-22 09:45:33]

https://codereview.chromium.org/1150913002/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/display/display_preferences_unittest.cc (right):

https://codereview.chromium.org/1150913002/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/display/display_preferences_unittest.cc:146:
StoreDisplayPropertyForPair(pair, key, new base::FundamentalValue(value));
This new base::FundamentalValue(value) leaks in some cases.

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x61740b in operator new(unsigned long)
(/tmp/run_tha_testilFjDM/out/Release/unit_tests+0x61740b)
    #1 0x40206a0 in chromeos::(anonymous
namespace)::DisplayPreferencesTest::StoreDisplayBoolPropertyForPair(std::pair\u003Clong,
long> const&, std::string const&, bool)
chrome/browser/chromeos/display/display_preferences_unittest.cc:146:44
    #2 0x401e200 in
chromeos::DisplayPreferencesTest_RestoreUnifiedMode_Test::TestBody()
chrome/browser/chromeos/display/display_preferences_unittest.cc:925:3
    #3 0x71007ec in HandleExceptionsInMethodIfSupported\u003Ctesting::Test,
void> testing/gtest/src/gtest.cc:2420:12
    #4 0x71007ec in testing::Test::Run() testing/gtest/src/gtest.cc:2436
    #5 0x7102c9d in testing::TestInfo::Run() testing/gtest/src/gtest.cc:2612:5
    #6 0x7103a1a in testing::TestCase::Run() testing/gtest/src/gtest.cc:2730:5
    #7 0x71178a3 in testing::internal::UnitTestImpl::RunAllTests()
testing/gtest/src/gtest.cc:4604:11
    #8 0x7116efb in
HandleExceptionsInMethodIfSupported\u003Ctesting::internal::UnitTestImpl, bool>
testing/gtest/src/gtest.cc:2420:12
    #9 0x7116efb in testing::UnitTest::Run() testing/gtest/src/gtest.cc:4222
    #10 0x135ebf06 in RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2326:10
    #11 0x135ebf06 in base::TestSuite::Run() base/test/test_suite.cc:228
    #12 0x135e3d3e in Run base/callback.h:396:12
    #13 0x135e3d3e in base::(anonymous
namespace)::LaunchUnitTestsInternal(base::Callback\u003Cint ()> const&, int,
bool, base::Callback\u003Cvoid ()> const&)
base/test/launcher/unit_test_launcher.cc:184
    #14 0x135e39d2 in base::LaunchUnitTests(int, char**, base::Callback\u003Cint
()> const&) base/test/launcher/unit_test_launcher.cc:423:10
    #15 0x72cc7bc in main chrome/test/base/run_all_unittests.cc:15:10
    #16 0x7fba24ddd76c in __libc_start_main
/build/buildd/eglibc-2.15/csu/libc-start.c:226

[battre, 2015-05-22 09:46:54]

A revert of this CL (patchset #5 id:140001) has been created in
https://codereview.chromium.org/1149063004/ by battre@chromium.org.

The reason for reverting is: Revert due to memory leak.

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x61740b in operator new(unsigned long)
(/tmp/run_tha_testilFjDM/out/Release/unit_tests+0x61740b)
    #1 0x40206a0 in chromeos::(anonymous
namespace)::DisplayPreferencesTest::StoreDisplayBoolPropertyForPair(std::pair\u003Clong,
long> const&, std::string const&, bool)
chrome/browser/chromeos/display/display_preferences_unittest.cc:146:44
    #2 0x401e200 in
chromeos::DisplayPreferencesTest_RestoreUnifiedMode_Test::TestBody()
chrome/browser/chromeos/display/display_preferences_unittest.cc:925:3.

[oshima, 2015-05-22 10:54:15]

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

[oshima, 2015-05-22 10:54:15]

The patchset sent to the CQ was uploaded after l-g-t-m from mukai@chromium.org
Link to the patchset: https://codereview.chromium.org/1150913002/#ps160001
(title: "fix leak")

[commit-bot: I haz the power, 2015-05-22 10:54:31]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1150913002/160001

[commit-bot: I haz the power, 2015-05-22 11:37:01]

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

[commit-bot: I haz the power, 2015-05-22 11:37:02]

Dry run: This issue passed the CQ dry run.

[oshima, 2015-05-22 17:52:39]

On 2015/05/22 11:37:02, I haz the power - commit-bot wrote:
> Dry run: This issue passed the CQ dry run.

failures in asan bot are unrelated.

[oshima, 2015-05-22 17:52:55]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2015-05-22 17:53:45]

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

[commit-bot: I haz the power, 2015-05-22 19:21:47]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2015-05-22 19:22:35]

Patchset 6 (id:??) landed as
https://crrev.com/bba2d991127c5cdb841e88df1c5613045cabc23c
Cr-Commit-Position: refs/heads/master@{#331149}