Move WebTextCheckClient reference from WebViewImpl to WebLocalFrameImpl

chrome/renderer/chrome_content_renderer_client.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/renderer/chrome_content_renderer_client.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 547 matching lines @@
   // ChromeRenderViewTests.
   if (subresource_filter_ruleset_dealer_) {
     new subresource_filter::SubresourceFilterAgent(
         render_frame, subresource_filter_ruleset_dealer_.get());
   }
 
   if (command_line->HasSwitch(switches::kInstantProcess) &&
       render_frame->IsMainFrame()) {
     new SearchBox(render_frame);
   }
+
+#if BUILDFLAG(ENABLE_SPELLCHECK)
+  // TODO(xiaochengh): Use a different SpellCheckProvider for each RenderFrame.
+  if (SpellCheckProvider* provider =
+          SpellCheckProvider::Get(render_frame->GetRenderView())) {

[Lei Zhang, 2017/04/05 20:20:44]

Looking at RenderFrameImpl::GetRenderView() and how it's used internally, it
should never returns a nullptr. So if all RenderView creation reaches
ChromeContentRendererClient::RenderViewCreated(), where you create the
SpellCheckProvider, then this should never return a nullptr either.

[Xiaocheng, 2017/04/05 20:26:15]

I found that for the first RenderFrame in a RenderView, RenderFrameCreated() is
called earlier than RenderViewCreated().

So when we reach here, it's possible that the SpellCheckProvider for the
RenderView hasn't been created yet.

[Lei Zhang, 2017/04/05 20:35:35]

Oh, we are in RenderFrameCreated(). I don't know enough about spell check to say
whether it's ok for the first (main?) RenderFrame to not have a client set. If
it must be set, maybe add a check to see if the client is set for the main frame
in RenderViewCreated() below and set it there instead after creating the
SpellCheckProvider?

It feels hacky, but I imagine the problem will go away once SpellCheckProvider
is a RenderFrameObserver.

[Xiaocheng, 2017/04/05 22:32:07]

Yes, each RenderFrame must have its client set. Do you mean:
- For a subframe, set its client in RenderFrameCreate()
- For the main frame, set its client in RenderViewCreated right after creating
the SpellCheckProvider?
Yeah it's hacky for now. Once SpellCheckProvider becomes RFO, we only need to
set the client in SpellCheckProvider::ctor and all hacks will be gone.

[Lei Zhang, 2017/04/05 22:38:19]

Based on your prior statements, my mental picture says it'll have to be done
this way.
It sounds like we are in agreement?

[Xiaocheng, 2017/04/05 22:43:34]

Yep :)

+    render_frame->GetWebFrame()->setTextCheckClient(provider);
+  }
+#endif
 }
 
 void ChromeContentRendererClient::RenderViewCreated(
     content::RenderView* render_view) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   ChromeExtensionsRendererClient::GetInstance()->RenderViewCreated(render_view);
 #endif
 #if BUILDFLAG(ENABLE_SPELLCHECK)
   new SpellCheckProvider(render_view, spellcheck_.get());
   new SpellCheckPanel(render_view);
@@ skipping 941 matching lines @@
 
   RecordYouTubeRewriteUMA(result);
   return corrected_url.ReplaceComponents(r);
 }
 
 std::unique_ptr<base::TaskScheduler::InitParams>
 ChromeContentRendererClient::GetTaskSchedulerInitParams() {
   return task_scheduler_util::
       GetRendererTaskSchedulerInitParamsFromCommandLine();
 }