Commit 92727047 authored by mgiuca's avatar mgiuca Committed by Commit bot

Added CHECKs for null pointers in ChromeAppSorting.

These are designed to give us more information about production-only
crashes occurring in ChromeAppSorting. These checks should help narrow
down the times at which extension_scoped_prefs_ is null.

Added TODOs to remove most of the checks later (although I plan to leave
one in as a safeguard).

BUG=476648

Review URL: https://codereview.chromium.org/1055453007

Cr-Commit-Position: refs/heads/master@{#326746}
parent f83db42d
...@@ -60,6 +60,10 @@ void ChromeAppSorting::SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) { ...@@ -60,6 +60,10 @@ void ChromeAppSorting::SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) {
extension_scoped_prefs_ = prefs; extension_scoped_prefs_ = prefs;
} }
void ChromeAppSorting::CheckExtensionScopedPrefs() const {
CHECK(extension_scoped_prefs_);
}
void ChromeAppSorting::SetExtensionSyncService( void ChromeAppSorting::SetExtensionSyncService(
ExtensionSyncService* extension_sync_service) { ExtensionSyncService* extension_sync_service) {
extension_sync_service_ = extension_sync_service; extension_sync_service_ = extension_sync_service;
...@@ -67,6 +71,7 @@ void ChromeAppSorting::SetExtensionSyncService( ...@@ -67,6 +71,7 @@ void ChromeAppSorting::SetExtensionSyncService(
void ChromeAppSorting::Initialize( void ChromeAppSorting::Initialize(
const extensions::ExtensionIdList& extension_ids) { const extensions::ExtensionIdList& extension_ids) {
CHECK(extension_scoped_prefs_);
InitializePageOrdinalMap(extension_ids); InitializePageOrdinalMap(extension_ids);
MigrateAppIndex(extension_ids); MigrateAppIndex(extension_ids);
...@@ -470,6 +475,9 @@ syncer::StringOrdinal ChromeAppSorting::GetMinOrMaxAppLaunchOrdinalsOnPage( ...@@ -470,6 +475,9 @@ syncer::StringOrdinal ChromeAppSorting::GetMinOrMaxAppLaunchOrdinalsOnPage(
void ChromeAppSorting::InitializePageOrdinalMap( void ChromeAppSorting::InitializePageOrdinalMap(
const extensions::ExtensionIdList& extension_ids) { const extensions::ExtensionIdList& extension_ids) {
// TODO(mgiuca): Added this CHECK to try and diagnose http://crbug.com/476648.
// Remove it after the investigation is concluded.
CHECK(extension_scoped_prefs_);
for (extensions::ExtensionIdList::const_iterator ext_it = for (extensions::ExtensionIdList::const_iterator ext_it =
extension_ids.begin(); ext_it != extension_ids.end(); ++ext_it) { extension_ids.begin(); ext_it != extension_ids.end(); ++ext_it) {
AddOrdinalMapping(*ext_it, AddOrdinalMapping(*ext_it,
......
...@@ -29,6 +29,7 @@ class ChromeAppSorting : public AppSorting { ...@@ -29,6 +29,7 @@ class ChromeAppSorting : public AppSorting {
// AppSorting implementation: // AppSorting implementation:
void SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) override; void SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) override;
void CheckExtensionScopedPrefs() const override;
void SetExtensionSyncService( void SetExtensionSyncService(
ExtensionSyncService* extension_sync_service) override; ExtensionSyncService* extension_sync_service) override;
void Initialize(const extensions::ExtensionIdList& extension_ids) override; void Initialize(const extensions::ExtensionIdList& extension_ids) override;
......
...@@ -27,11 +27,17 @@ class AppSorting { ...@@ -27,11 +27,17 @@ class AppSorting {
// caller. // caller.
virtual void SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) = 0; virtual void SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) = 0;
// CHECKs that SetExtensionScopedPrefs has been called with a non-null object.
// TODO(mgiuca): Added this to try and diagnose http://crbug.com/476648.
// Remove it after the investigation is concluded.
virtual void CheckExtensionScopedPrefs() const = 0;
// Sets up the ExtensionSyncService to inform of changes that require syncing. // Sets up the ExtensionSyncService to inform of changes that require syncing.
virtual void SetExtensionSyncService( virtual void SetExtensionSyncService(
ExtensionSyncService* extension_sync_service) = 0; ExtensionSyncService* extension_sync_service) = 0;
// Properly initializes internal values that require |extension_ids|. // Properly initializes internal values that require |extension_ids|.
// SetExtensionScopedPrefs must have been called prior to this.
virtual void Initialize(const extensions::ExtensionIdList& extension_ids) = 0; virtual void Initialize(const extensions::ExtensionIdList& extension_ids) = 0;
// Resolves any conflicts the might be created as a result of syncing that // Resolves any conflicts the might be created as a result of syncing that
......
...@@ -1866,7 +1866,11 @@ ExtensionPrefs::ExtensionPrefs( ...@@ -1866,7 +1866,11 @@ ExtensionPrefs::ExtensionPrefs(
app_sorting_(app_sorting.Pass()), app_sorting_(app_sorting.Pass()),
time_provider_(time_provider.Pass()), time_provider_(time_provider.Pass()),
extensions_disabled_(extensions_disabled) { extensions_disabled_(extensions_disabled) {
// TODO(mgiuca): Added these checks to try and diagnose
// http://crbug.com/476648. Remove them after the investigation is concluded.
CHECK(this);
app_sorting_->SetExtensionScopedPrefs(this); app_sorting_->SetExtensionScopedPrefs(this);
app_sorting_->CheckExtensionScopedPrefs();
MakePathsRelative(); MakePathsRelative();
// Ensure that any early observers are watching before prefs are initialized. // Ensure that any early observers are watching before prefs are initialized.
......
...@@ -26,6 +26,9 @@ NullAppSorting::~NullAppSorting() { ...@@ -26,6 +26,9 @@ NullAppSorting::~NullAppSorting() {
void NullAppSorting::SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) { void NullAppSorting::SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) {
} }
void NullAppSorting::CheckExtensionScopedPrefs() const {
}
void NullAppSorting::SetExtensionSyncService( void NullAppSorting::SetExtensionSyncService(
ExtensionSyncService* extension_sync_service) { ExtensionSyncService* extension_sync_service) {
} }
......
...@@ -19,6 +19,7 @@ class NullAppSorting : public AppSorting { ...@@ -19,6 +19,7 @@ class NullAppSorting : public AppSorting {
// AppSorting overrides: // AppSorting overrides:
void SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) override; void SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) override;
void CheckExtensionScopedPrefs() const override;
void SetExtensionSyncService( void SetExtensionSyncService(
ExtensionSyncService* extension_sync_service) override; ExtensionSyncService* extension_sync_service) override;
void Initialize(const ExtensionIdList& extension_ids) override; void Initialize(const ExtensionIdList& extension_ids) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment