Colin is just about to change this, I think. (Not to dump it on you, but perhaps if you improve the registration part you mentioned, this crash will be fixed.)

I think this crash is orthogonal to the changes that I'm making. It seems like ViewAndroid::AcquireAnchorView() is DCHECKing in deep internals when called on a valid ViewAndroid pointer; the DCHECK looks like it's here: https://cs.chromium.org/chromium/src/base/android/jni_weak_ref.cc?q=jni_weak_ref.cc&sq=package:chromium&dr&l=36, called from here: https://cs.chromium.org/chromium/src/ui/android/view_android.cc?q=view_android.cc&sq=package:chromium&dr&l=25, called from here: https://cs.chromium.org/chromium/src/ui/android/view_android.cc?q=view_android.cc&sq=package:chromium&dr&l=110. I'm not familiar enough with Java or this code to understand what assumption is being violated; perhaps boliu@ is?

Yep, blundell is all correct in #2.

In ViewAndroid::AcquireAnchorView, there is already a check for the null delegate already, but not for the view, so need to add that. Jinsuk, wanna take care of that?

On the other hand, that DCHECK seems really odd... it should be totally legal to assign null to a reference, even if it's like bad form. Is there any technical reason why we do that? torne?

It is indeed legal to create a weak reference to null as far as I can see (it will obviously always be null when you look at it again later, but weak references might become null any time anyway, so yaknow). I think the code is just being defensive and assuming nobody would actually do that?

If you think it's useful we could probably remove that DCHECK.

Just to clarify - it's okay to have a nulled-out view in ScopedAnchorView. It only cares about the situation where we have a valid view but nulled-out delegate, which is checked at https://cs.chromium.org/chromium/src/ui/android/view_android.cc?l=27 since it wouldn't be possible to pass the valid view up to Java layer later. I think we only need to modify JavaObjectWeakGlobalRef ctr to fix the crash.

yeah, removing the DCHECK is fine

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48678a7b6e9087357262f8acee9df186b337d706

commit 48678a7b6e9087357262f8acee9df186b337d706
Author: jinsukkim <jinsukkim@chromium.org>
Date: Wed Oct 05 13:41:18 2016

Remove null checking in JavaObjectWeakGlobalRef ctr

It is legal to create a weak reference to null. Removes the defensive
null checking being done in the constructors.

BUG= 649407 

Review-Url: https://codereview.chromium.org/2389403002
Cr-Commit-Position: refs/heads/master@{#423143}

[modify] https://crrev.com/48678a7b6e9087357262f8acee9df186b337d706/base/android/jni_weak_ref.cc

Thanks jinsukkim!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48678a7b6e9087357262f8acee9df186b337d706

commit 48678a7b6e9087357262f8acee9df186b337d706
Author: jinsukkim <jinsukkim@chromium.org>
Date: Wed Oct 05 13:41:18 2016

Remove null checking in JavaObjectWeakGlobalRef ctr

It is legal to create a weak reference to null. Removes the defensive
null checking being done in the constructors.

BUG= 649407 

Review-Url: https://codereview.chromium.org/2389403002
Cr-Commit-Position: refs/heads/master@{#423143}

[modify] https://crrev.com/48678a7b6e9087357262f8acee9df186b337d706/base/android/jni_weak_ref.cc

[Automated comment] removing mislabelled merge-merged-2840

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599

commit 73b9848f38e00bf1b9f48cfa2da7f8cda7c41599
Author: rockot <rockot@chromium.org>
Date: Wed Feb 15 18:58:02 2017

Make browser process a singleton service

Changes the primordial browser process service manager connection
to identify as a singleton service named content_packaged_services.
This is used to package any existing global services previously
associated with the global root content_browser connection.

The root content_browser connection still exists in order to serve
as a client process host for non-renderer child processes.

One result of this change is that no services other than content_*
are allowed to connect directly to content_browser, in order to avoid
racing with the (now asynchronous) content_browser instance creation
by ServiceManagerContext. As such, code which previously connected to
content_browser has also been fixed to do something less weird. To wit:

 - An "ash" service is now embedded in the browser process when not
   running in mash mode - this allows simplification of ash client code
   as there is no longer a need to switch the target service name at
   runtime when connecting to ash interfaces
 - A new "chrome" service is packaged in content_packaged_services by
   within Chrome. This hosts a mash::mojom::Launchable interface, allowing
   Chrome to be launched within mash by connecting to "chrome"
 - ChromeInterfaceFactory is deleted

Somewhat unrelated but because this CL exposed a timing issue between packaged
service connection and main thread browser initialization, this also removes the
unnecessary blocking of startup on service manager connection (see change in
chrome_browser_main_extra_parts_views.cc.)

BUG= 649407 ,594852
TEST=chrome --mash functions normally, standalone mash runner functions normally with chrome, chrome non-mash functions normally, browser_tests work with and without --run-in-mash
TBR=tsepez@chromium.org

Review-Url: https://codereview.chromium.org/2695803004
Cr-Commit-Position: refs/heads/master@{#450750}

[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/ash/mus/manifest.json
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/app/BUILD.gn
[add] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/app/chrome_manifest.json
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/app/mash/BUILD.gn
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/app/mash/mash_runner.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/DEPS
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/browser_resources.grd
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/chrome_content_browser_manifest_overlay.json
[add] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/chrome_content_packaged_services_manifest_overlay.json
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/chromeos/BUILD.gn
[delete] https://crrev.com/5e8ee75a86c1c36b53c1a6b5ec28d89b7bb6d4ad/chrome/browser/chromeos/chrome_interface_factory.cc
[delete] https://crrev.com/5e8ee75a86c1c36b53c1a6b5ec28d89b7bb6d4ad/chrome/browser/chromeos/chrome_interface_factory.h
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/prefs/preferences_connection_manager.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/ui/ash/ash_util.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/ui/ash/ash_util.h
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/browser/ui/views/chrome_browser_main_extra_parts_views.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/test/BUILD.gn
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/chrome/test/base/mojo_test_connector.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/BUILD.gn
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/browser/service_manager/service_manager_context.h
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/common/service_manager/service_manager_connection_impl.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/common/service_manager/service_manager_connection_impl.h
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/content_resources.grd
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/public/app/BUILD.gn
[add] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/public/app/mojo/content_packaged_services_manifest.json
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/public/common/service_manager_connection.h
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/content/public/common/service_names.mojom
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/mash/BUILD.gn
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/mash/session/session.cc
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/services/service_manager/public/cpp/interface_registry.h
[modify] https://crrev.com/73b9848f38e00bf1b9f48cfa2da7f8cda7c41599/services/service_manager/public/cpp/lib/interface_registry.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afc42ced3ccb6d8f6ce82ee7fe358ca299f28f9f

commit afc42ced3ccb6d8f6ce82ee7fe358ca299f28f9f
Author: slan <slan@chromium.org>
Date: Tue Mar 07 22:56:40 2017

[Chromecast] Update manifests to use content_packaged_services.

Since crrrev.com/2695803004, all services that don't require a
BrowserContext should be packaged in |content_packaged_services|.
Cast-specific services are all registered on this connector, so update
each manifest and comments.

BUG= 649407 
Test: cast_shell_internal_browsertest

Review-Url: https://codereview.chromium.org/2731913004
Cr-Commit-Position: refs/heads/master@{#455270}

[modify] https://crrev.com/afc42ced3ccb6d8f6ce82ee7fe358ca299f28f9f/chromecast/browser/BUILD.gn
[modify] https://crrev.com/afc42ced3ccb6d8f6ce82ee7fe358ca299f28f9f/chromecast/browser/cast_browser_resources.grd
[modify] https://crrev.com/afc42ced3ccb6d8f6ce82ee7fe358ca299f28f9f/chromecast/browser/cast_content_browser_client.cc
[add] https://crrev.com/afc42ced3ccb6d8f6ce82ee7fe358ca299f28f9f/chromecast/browser/cast_content_packaged_services_manifest_overlay.json