mus: aura::Env should be destroyed last in the mus client.

mus: aura::Env should be destroyed last in the mus client.

BUG=674803
Committed: https://crrev.com/2626c1919f27753ff20c855f4136f6414264c956
Cr-Commit-Position: refs/heads/master@{#440316}

[sadrul, 2016-12-21 03:48:35]

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

[commit-bot: I haz the power, 2016-12-21 03:48:47]

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, 2016-12-21 04:00:02]

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

[commit-bot: I haz the power, 2016-12-21 04:00:02]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[sadrul, 2016-12-21 04:05:28]

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

[commit-bot: I haz the power, 2016-12-21 04:05:52]

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, 2016-12-21 04:59:05]

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

[commit-bot: I haz the power, 2016-12-21 04:59:06]

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

[sadrul, 2016-12-21 05:12:17]

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

[commit-bot: I haz the power, 2016-12-21 05:12:27]

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, 2016-12-21 06:32:54]

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

[commit-bot: I haz the power, 2016-12-21 06:32:55]

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

[sadrul, 2016-12-21 16:04:20]

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

[commit-bot: I haz the power, 2016-12-21 16:04:38]

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, 2016-12-21 17:38:42]

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

[commit-bot: I haz the power, 2016-12-21 17:38:43]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-12-21 17:40:59]

sadrul@chromium.org changed reviewers:
+ msw@chromium.org

[sadrul, 2016-12-21 17:43:52]

This CL also needs https://codereview.chromium.org/2596903002/ since we create
the allocator for each test.

[sadrul, 2016-12-21 17:46:15]

On 2016/12/21 17:43:52, sadrul wrote:
> This CL also needs https://codereview.chromium.org/2596903002/ since we create
> the allocator for each test.

Sorry, wrong CL. This CL does not depend on the discardable-memory allocator
fix.

[msw, 2016-12-21 17:56:35]

rubber stamp lgtm with q and optional nit.

https://codereview.chromium.org/2591863004/diff/60001/content/browser/browser...
File content/browser/browser_main_loop.cc (left):

https://codereview.chromium.org/2591863004/diff/60001/content/browser/browser...
content/browser/browser_main_loop.cc:1210: env_.reset();
aura::Env doesn't have any deps on objects explicitly destroyed below?
(probably hard to ascertain, keep an eye out for shutdown crashes?)

https://codereview.chromium.org/2591863004/diff/60001/ui/aura/mus/focus_synch...
File ui/aura/mus/focus_synchronizer.cc (right):

https://codereview.chromium.org/2591863004/diff/60001/ui/aura/mus/focus_synch...
ui/aura/mus/focus_synchronizer.cc:25: if (env_) {
optional nit: don't cache Env::GetInstance, just call that in the ctor and dtor?

[sadrul, 2016-12-21 18:01:22]

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

[commit-bot: I haz the power, 2016-12-21 18:02:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-12-21 18:03:36]

https://codereview.chromium.org/2591863004/diff/60001/content/browser/browser...
File content/browser/browser_main_loop.cc (left):

https://codereview.chromium.org/2591863004/diff/60001/content/browser/browser...
content/browser/browser_main_loop.cc:1210: env_.reset();
On 2016/12/21 17:56:35, msw wrote:
> aura::Env doesn't have any deps on objects explicitly destroyed below?
> (probably hard to ascertain, keep an eye out for shutdown crashes?)

Yep. I will keep an eye on it (the trybots seem happy though)

https://codereview.chromium.org/2591863004/diff/60001/ui/aura/mus/focus_synch...
File ui/aura/mus/focus_synchronizer.cc (right):

https://codereview.chromium.org/2591863004/diff/60001/ui/aura/mus/focus_synch...
ui/aura/mus/focus_synchronizer.cc:25: if (env_) {
On 2016/12/21 17:56:35, msw wrote:
> optional nit: don't cache Env::GetInstance, just call that in the ctor and
dtor?

Nice! Done. Thanks!

[commit-bot: I haz the power, 2016-12-21 19:39:47]

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

[commit-bot: I haz the power, 2016-12-21 19:39:48]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-12-22 00:42:48]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2016-12-22 00:42:49]

+ben@ for content/browser change.

[Ben Goodger (Google), 2016-12-22 00:56:30]

lgtm

[sadrul, 2016-12-22 01:12:37]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-12-22 01:12:38]

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

[commit-bot: I haz the power, 2016-12-22 01:13:17]

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, 2016-12-22 01:23:03]

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

[commit-bot: I haz the power, 2016-12-22 01:23:08]

Failed to apply patch for ui/views/mus/mus_client.cc:
While running git apply --index -p1;
  error: patch failed: ui/views/mus/mus_client.cc:130
  error: ui/views/mus/mus_client.cc: patch does not apply

Patch:       ui/views/mus/mus_client.cc
Index: ui/views/mus/mus_client.cc
diff --git a/ui/views/mus/mus_client.cc b/ui/views/mus/mus_client.cc
index
575cf995b8154b842e0513fe5c0337ca123d2d7a..17f7d750eb2138192eb341ee1815782a988759b3
100644
--- a/ui/views/mus/mus_client.cc
+++ b/ui/views/mus/mus_client.cc
@@ -64,6 +64,7 @@ MusClient::MusClient(service_manager::Connector* connector,
                      bool create_wm_state)
     : identity_(identity) {
   DCHECK(!instance_);
+  DCHECK(aura::Env::GetInstance());
   instance_ = this;
 
   if (!io_task_runner) {
@@ -130,6 +131,7 @@ MusClient::~MusClient() {
 
   DCHECK_EQ(instance_, this);
   instance_ = nullptr;
+  DCHECK(aura::Env::GetInstance());
 }
 
 // static

[sadrul, 2016-12-22 01:32:36]

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

[commit-bot: I haz the power, 2016-12-22 01:32:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-12-22 02:14:14]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-12-22 02:14:14]

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

[commit-bot: I haz the power, 2016-12-22 02:14:21]

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, 2016-12-22 02:33:45]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1482372854413900,
"parent_rev": "ca13ae5f3721c052661e69d8352f4fd8611cd54c", "commit_rev":
"9a68972d82466118bf8dcdea6f8d01efdbb0ac91"}

[commit-bot: I haz the power, 2016-12-22 02:34:11]

Description was changed from

==========
mus: aura::Env should be destroyed last in the mus client.

BUG=674803
==========

to

==========
mus: aura::Env should be destroyed last in the mus client.

BUG=674803

Review-Url: https://codereview.chromium.org/2591863004
==========

[commit-bot: I haz the power, 2016-12-22 02:34:12]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-12-22 02:35:35]

Description was changed from

==========
mus: aura::Env should be destroyed last in the mus client.

BUG=674803

Review-Url: https://codereview.chromium.org/2591863004
==========

to

==========
mus: aura::Env should be destroyed last in the mus client.

BUG=674803

Committed: https://crrev.com/2626c1919f27753ff20c855f4136f6414264c956
Cr-Commit-Position: refs/heads/master@{#440316}
==========

[commit-bot: I haz the power, 2016-12-22 02:35:36]

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