GetActiveUserProfile() should not be called before profile is initialized.

GetActiveUserProfile() should not be called before profile is initialized.

During extension initialization we should not rely on GetActiveUserProfile().

BUG=397971
TEST=manual

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287371

[Alexander Alekseev, 2014-08-01 16:22:14]

Please review.

[Nikita (slow), 2014-08-01 16:24:52]

lgtm

[Alexander Alekseev, 2014-08-01 18:13:32]

The second version gets correct signin profile.

[Nikita (slow), 2014-08-01 20:31:10]

lgtm

[Shu Chen, 2014-08-01 22:11:07]

This cl is incomplete to fix the bug. Please refer to my cl
https://codereview.chromium.org/430243003/.

InputImeEventRouter::RegisterIme() calls InputMethodEngine::Initialize() calls
InputMethodManagerImpl::AddInputMethodExtension(), which also calls
GetActiveUserProfile() which also returns wrong profile.

[Alexander Alekseev, 2014-08-02 04:51:48]

On 2014/08/01 22:11:07, Shu Chen wrote:
> This cl is incomplete to fix the bug. Please refer to my cl
> https://codereview.chromium.org/430243003/.
> 
> InputImeEventRouter::RegisterIme() calls InputMethodEngine::Initialize() calls
> InputMethodManagerImpl::AddInputMethodExtension(), which also calls
> GetActiveUserProfile() which also returns wrong profile.

Yes, you're right.

I've removed IME::GetProfile() completely.
Now all callers supply valid Profile* to IME.

I would say it looks like temporary fix, as I would remove all Profile* from IME
completely.
But it works and doesn't introduce new structures.

What do you think?

[Shu Chen, 2014-08-02 08:36:01]

This CL makes all profile instances passing to IMF, which may potentially break
things.

I plan to make a refactoring to IMF less depends on profile.
So can you please just make sure passing profile to
InputImeEventRouter::RegisterIme() and
InputMethodManagerImpl::AddInputMethodExtension()?

I think it should be enough to fix the bug.

Thanks,
Shu

[Alexander Alekseev, 2014-08-02 13:50:26]

On 2014/08/02 08:36:01, Shu Chen wrote:
> This CL makes all profile instances passing to IMF, which may potentially
break
> things.
> 
> I plan to make a refactoring to IMF less depends on profile.
> So can you please just make sure passing profile to
> InputImeEventRouter::RegisterIme() and
> InputMethodManagerImpl::AddInputMethodExtension()?
> 
> I think it should be enough to fix the bug.
> 
> Thanks,
> Shu

Done. Please review.

[Alexander Alekseev, 2014-08-02 13:55:33]

+ yukishiino@: Please review:

chromeos/ime/input_method_manager.h

[Yuki, 2014-08-02 14:07:49]

lgtm for chromeos/ime/input_method_manager.h

[Shu Chen, 2014-08-02 23:15:39]

lgtm

[Alexander Alekseev, 2014-08-02 23:32:36]

Nikita, please, take another look.

[Nikita (slow), 2014-08-04 12:51:39]

lgtm

https://codereview.chromium.org/428783008/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/428783008/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:870: Profile*
InputMethodManagerImpl::GetProfile() const {
This method is a potential source of similar bugs. Please add comments to it in
the header.

[Alexander Alekseev, 2014-08-04 16:24:13]

https://codereview.chromium.org/428783008/diff/80001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.cc (right):

https://codereview.chromium.org/428783008/diff/80001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.cc:870: Profile*
InputMethodManagerImpl::GetProfile() const {
On 2014/08/04 12:51:39, Nikita Kostylev wrote:
> This method is a potential source of similar bugs. Please add comments to it
in
> the header.

Done.

[Alexander Alekseev, 2014-08-04 16:24:17]

The CQ bit was checked by alemate@chromium.org

[commit-bot: I haz the power, 2014-08-04 16:25:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/alemate@chromium.org/428783008/100001

[commit-bot: I haz the power, 2014-08-04 18:23:00]

Change committed as 287371