[ios] Fix clean shutdown of Chrome on iOS.

[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=722879
Review-Url: https://codereview.chromium.org/2883323002
Cr-Commit-Position: refs/heads/master@{#472437}
Committed: https://chromium.googlesource.com/chromium/src/+/8ac159105b78c8987a16faa4113d18b37682566b

[sdefresne, 2017-05-16 16:15:51]

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

[sdefresne, 2017-05-16 16:16:01]

sdefresne@chromium.org changed reviewers:
+ rohitrao@chromium.org

[sdefresne, 2017-05-16 16:16:02]

Please take a look.

[commit-bot: I haz the power, 2017-05-16 16:16:33]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sdefresne, 2017-05-16 16:16:47]

Description was changed from

==========
[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=None
==========

to

==========
[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=None
==========

[sdefresne, 2017-05-16 16:16:47]

sdefresne@chromium.org changed reviewers:
+ stkhapugin@chromium.org

[sdefresne, 2017-05-16 16:17:40]

rohitrao: please take a look as this fix crashes / DCHECK failures during
shutdown of the app.

stk: FYI, some of them were introduced by the ARC migration (in particular the
changes in topsites_spotlight_manager.mm).

[sdefresne, 2017-05-16 16:25:28]

Description was changed from

==========
[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=None
==========

to

==========
[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=722879
==========

[commit-bot: I haz the power, 2017-05-16 16:40:26]

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

[commit-bot: I haz the power, 2017-05-16 16:40:27]

Dry run: This issue passed the CQ dry run.

[rohitrao (ping after 24h), 2017-05-16 16:41:02]

https://codereview.chromium.org/2883323002/diff/1/ios/chrome/app/main_control...
File ios/chrome/app/main_controller.mm (right):

https://codereview.chromium.org/2883323002/diff/1/ios/chrome/app/main_control...
ios/chrome/app/main_controller.mm:956: _spotlightManager.reset();
There's no guarantee that this call will destroy the spotlight manager.  Is that
something we should handle?

[sdefresne, 2017-05-16 16:42:41]

On 2017/05/16 16:41:02, rohitrao (ping after 24h) wrote:
>
https://codereview.chromium.org/2883323002/diff/1/ios/chrome/app/main_control...
> File ios/chrome/app/main_controller.mm (right):
> 
>
https://codereview.chromium.org/2883323002/diff/1/ios/chrome/app/main_control...
> ios/chrome/app/main_controller.mm:956: _spotlightManager.reset();
> There's no guarantee that this call will destroy the spotlight manager.  Is
that
> something we should handle?

Probably, but I'd say this can be done in a followup.

[sdefresne, 2017-05-16 16:44:40]

https://codereview.chromium.org/2883323002/diff/1/ios/chrome/app/main_control...
File ios/chrome/app/main_controller.mm (right):

https://codereview.chromium.org/2883323002/diff/1/ios/chrome/app/main_control...
ios/chrome/app/main_controller.mm:957: _browserViewWrangler.reset();
There is no guarantee that this will destroy _browserViewWrangler either :-)

[rohitrao (ping after 24h), 2017-05-16 16:58:44]

lgtm

[stkhapugin, 2017-05-17 12:27:15]

lgtm, thanks for looking into this

[sdefresne, 2017-05-17 13:09:51]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2017-05-17 13:09:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
stkhapugin@chromium.org, rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/2883323002/#ps20001
(title: "Rebase on origin/master.")

[commit-bot: I haz the power, 2017-05-17 13:10:04]

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-17 13:21:12]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1495026591467210,
"parent_rev": "b59e8df0bf51b0de1d7924d96bfebfbb2705ddc3", "commit_rev":
"8ac159105b78c8987a16faa4113d18b37682566b"}

[commit-bot: I haz the power, 2017-05-17 13:21:26]

Description was changed from

==========
[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=722879
==========

to

==========
[ios] Fix clean shutdown of Chrome on iOS.

Drop the references to SpotlightManager and BrowserViewWrangler in
MainController -stopChromeMain before destroying IOSChromeMain as
IOSChromeMain leads to the destruction of ChromeBrowserState and
both SpotlightManager and BrowserViewWrangler must not outlive the
ChromeBrowserState.

Fix SpotlightTopSitesBridge destructor by keeping a reference to
history::TopSites as __weak pointer are set to nil before the ivar
are destroyed, thus owner_->topSites is null by that point.

Fix ToolsMenuButtonObserverBridge to destroy the observer bridge
when setting _model to null to avoid DCHECK.

BUG=722879

Review-Url: https://codereview.chromium.org/2883323002
Cr-Commit-Position: refs/heads/master@{#472437}
Committed:
https://chromium.googlesource.com/chromium/src/+/8ac159105b78c8987a16faa4113d...
==========

[commit-bot: I haz the power, 2017-05-17 13:21:30]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8ac159105b78c8987a16faa4113d...