Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2531023002:
Don't create profile in HttpsEngagementMetricsProvider (Closed)
DescriptionDon't create profile in HttpsEngagementMetricsProvider
`ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown.
BUG=
Committed: https://crrev.com/5fccf7ca2d9ce01847d0d6459db73636d537dce3
Cr-Commit-Position: refs/heads/master@{#434916}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Add a comment #Messages
Total messages: 18 (8 generated)
Description was changed from ========== Don't create profile in HttpsEngagementMetricsProvider `ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown. BUG= ========== to ========== Don't create profile in HttpsEngagementMetricsProvider `ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown. BUG= ==========
gurrrik@yandex-team.ru changed reviewers: + holte@chromium.org
On 2016/11/25 11:34:56, gurrrik wrote: > mailto:gurrrik@yandex-team.ru changed reviewers: > + mailto:holte@chromium.org Please take a look.
asvitkine@chromium.org changed reviewers: + asvitkine@chromium.org - holte@chromium.org
https://codereview.chromium.org/2531023002/diff/1/chrome/browser/metrics/http... File chrome/browser/metrics/https_engagement_metrics_provider.cc (right): https://codereview.chromium.org/2531023002/diff/1/chrome/browser/metrics/http... chrome/browser/metrics/https_engagement_metrics_provider.cc:22: Profile* profile = profile_manager->GetProfileByPath( Please add a comment about this.
On 2016/11/25 18:42:02, Alexei Svitkine (slow) wrote: > https://codereview.chromium.org/2531023002/diff/1/chrome/browser/metrics/http... > File chrome/browser/metrics/https_engagement_metrics_provider.cc (right): > > https://codereview.chromium.org/2531023002/diff/1/chrome/browser/metrics/http... > chrome/browser/metrics/https_engagement_metrics_provider.cc:22: Profile* profile > = profile_manager->GetProfileByPath( > Please add a comment about this. Done.
lgtm
holte@chromium.org changed reviewers: + holte@chromium.org
lgtm
The CQ bit was checked by gurrrik@yandex-team.ru
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480397563511720,
"parent_rev": "046203e17617808b5de14e0ac244657e6952cd1d", "commit_rev":
"41cd7c95eb829d845b43ade63a29db08100cf4f2"}
Message was sent while issue was closed.
Description was changed from ========== Don't create profile in HttpsEngagementMetricsProvider `ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown. BUG= ========== to ========== Don't create profile in HttpsEngagementMetricsProvider `ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown. BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Don't create profile in HttpsEngagementMetricsProvider `ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown. BUG= ========== to ========== Don't create profile in HttpsEngagementMetricsProvider `ProfileManager::GetLastUsedProfile` creates a profile if it does not exist. `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` can be called in `PostMainMessageLoopRun`, and it will try to create a profile, which is not what you'd want during shutdown. BUG= Committed: https://crrev.com/5fccf7ca2d9ce01847d0d6459db73636d537dce3 Cr-Commit-Position: refs/heads/master@{#434916} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/5fccf7ca2d9ce01847d0d6459db73636d537dce3 Cr-Commit-Position: refs/heads/master@{#434916}
Message was sent while issue was closed.
On 2016/11/29 06:54:19, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as > https://crrev.com/5fccf7ca2d9ce01847d0d6459db73636d537dce3 > Cr-Commit-Position: refs/heads/master@{#434916} Hi, I just ran into this CL while looking at https://crbug.com/671579. What was the context of this change? Was it a crash?
Message was sent while issue was closed.
On 2016/12/21 19:56:31, Mustafa Emre Acer wrote: > On 2016/11/29 06:54:19, commit-bot: I haz the power wrote: > > Patchset 2 (id:??) landed as > > https://crrev.com/5fccf7ca2d9ce01847d0d6459db73636d537dce3 > > Cr-Commit-Position: refs/heads/master@{#434916} > > Hi, I just ran into this CL while looking at https://crbug.com/671579. What was > the context of this change? Was it a crash? Hi! Yes, it was a crash which was caused by this check: https://chromium.googlesource.com/chromium/src/+/master/content/browser/rende... An attempt to create `RenderProcessHostImpl` happened when creating a profile, which in turn happened in `HttpsEngagementMetricsProvider::ProvideGeneralMetrics` during browser shutdown. Unfortunately I don't have access to the issue you mentioned, so I can't comment on that. |
