Closed Bug 1395443 Opened 2 years ago Closed 2 years ago

DictionaryFetcher::Fetch should initializate nsIContentPrefService2 lazy

Categories

(Core :: Editor, enhancement, P3)

Unspecified
All
enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: m_kato, Assigned: m_kato)

Details

Attachments

(1 file)

http://bit.ly/2el4E9m

When using contenteditable with spellchecker (it is default), spellchecker will be initialized by mozilla::EditorEventListener::Focus.  This method will call nsEditorSpellCheck::UpdateCurrentDictionary.  But this method seems to be slow.

DictionaryFetcher::Fetch gets nsIContentPrefService2 that is implemented by JavaScript, so we should create nsIContentPrefService2 out of Fetch method.
Comment on attachment 8904216 [details]
Bug 1395443 - DictionaryFetcher::Fetch should initializate nsIContentPrefService2 by idle thread.

https://reviewboard.mozilla.org/r/175998/#review181008

::: editor/composer/nsEditorSpellCheck.cpp:171
(Diff revision 1)
> +  NS_IMETHOD Run() override
> +  {
> +    nsCOMPtr<nsIContentPrefService2> contentPrefService =
> +      do_GetService(NS_CONTENT_PREF_SERVICE_CONTRACTID);
> +    if (!contentPrefService) {
> +      mCallback->HandleError(NS_ERROR_NOT_AVAILABLE);
> +      return NS_OK;
> +    }
> +
> +    nsAutoCString docUriSpec;
> +    nsresult rv = mDocUri->GetSpec(docUriSpec);
> +    if (NS_FAILED(rv)) {
> +      mCallback->HandleError(rv);
> +      return NS_OK;
> +    }
> +
> +    rv = contentPrefService->GetByDomainAndName(
> +                               NS_ConvertUTF8toUTF16(docUriSpec),
> +                               CPS_PREF_NAME, mLoadContext,
> +                               mCallback);
> +    if (NS_FAILED(rv)) {
> +      mCallback->HandleError(rv);
> +      return NS_OK;
> +    }
> +    return NS_OK;

Hmm, so, this method may run even after the document is unloaded, the spellchecker is disabled or something...

At least cannot we check if the editor is destroyed with (grabbing DictionaryFetcher instead of mCallback, then,) DictionaryFetcher->mSpellCheck->mEditor->AsTextEditor()->Destroyed()? Then, we could cancel the expensive jobs if <input>/<textarea>/<iframe> with editable document are removed quickly.

Or it might be better to store editor and retrieve mLoadContext with DictionaryFetcher::GetLoadContext() in this method instead of DictionaryFetcher::Fetch()?
Sigh, I realized that if I don't mark it r-, new review request won't come with email...
Comment on attachment 8904216 [details]
Bug 1395443 - DictionaryFetcher::Fetch should initializate nsIContentPrefService2 by idle thread.

https://reviewboard.mozilla.org/r/175998/#review181662
Attachment #8904216 - Flags: review?(masayuki) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/4b5b2e44447d
DictionaryFetcher::Fetch should initializate nsIContentPrefService2 by idle thread. r=masayuki
https://hg.mozilla.org/mozilla-central/rev/4b5b2e44447d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.