Populate the distilled content slightly earlier

Before this CL, the distilled content is injected on DidFinishLoad(),
or when the distillation finishes if it's later than DidFinishLoad().
When the DidFinishLoad() event propagates to TalkBack, the distilled
content is usually not there yet, so TalkBack would announce the
content of the place-holding page.

This CL populates the distilled content in DocumentLoadedInFrame()
instead of DidFinishLoad(). DocumentLoadedInFrame() is late enough to
execute JavaScript, and is early enough so that it's more likely that
the title and content can be picked up by TalkBack instead of the
placeholder.

If distillation is finished by DocumentLoadedInFrame(), onload() event
would also be delayed, so that the accessibility focus is more likely
to be on the web content. Otherwise, the focus is usually on the close
button of the CustomTab (CCT), or nowhere.

Note that this CL doesn't guarantee the distilled title is announced
by TalkBack. If distillation finishes later than
DocumentLoadedInFrame(), or if for some reason the accessibility focus
is on the close button of the CCT, the title would go unannounced like
before.

Bug: 811417, 803474
Change-Id: Iaf2ff988b427ca9af582622597b505e7a8aca8e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1791048Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Commit-Queue: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697074}

chrome/browser/dom_distiller/tab_utils_browsertest.cc

@@ -86,8 +86,10 @@ class DistilledPageObserver : public NavigationObserver {
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override {
     if (!render_frame_host->GetParent() &&
-        validated_url.scheme() == kDomDistillerScheme)
+        validated_url.scheme() == kDomDistillerScheme) {
       loaded_distiller_page_ = true;
+      MaybeNotifyLoaded();
+    }
   }
 
   void TitleWasSet(content::NavigationEntry* entry) override {
@@ -95,14 +97,19 @@ class DistilledPageObserver : public NavigationObserver {
     // and once when the distillation has finished. Watch for the second time
     // as a signal that the JavaScript that sets the content has run.
     title_set_count_++;
-    if (title_set_count_ >= 2 && loaded_distiller_page_) {
-      new_url_loaded_runner_.QuitClosure().Run();
-    }
+    MaybeNotifyLoaded();
   }
 
  private:
   int title_set_count_;
   bool loaded_distiller_page_;
+
+  // DidFinishLoad() can come after the two title settings.
+  void MaybeNotifyLoaded() {
+    if (title_set_count_ >= 2 && loaded_distiller_page_) {
+      new_url_loaded_runner_.QuitClosure().Run();
+    }
+  }
 };
 
 class DomDistillerTabUtilsBrowserTest : public InProcessBrowserTest {

components/dom_distiller/content/browser/dom_distiller_viewer_source.cc

@@ -65,8 +65,8 @@ class DomDistillerViewerSource::RequestViewerHandle
       content::NavigationHandle* navigation_handle) override;
   void RenderProcessGone(base::TerminationStatus status) override;
   void WebContentsDestroyed() override;
-  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
-                     const GURL& validated_url) override;
+  void DocumentLoadedInFrame(
+      content::RenderFrameHost* render_frame_host) override;
   void OnInterfaceRequestFromFrame(
       content::RenderFrameHost* render_frame_host,
       const std::string& interface_name,
@@ -179,14 +179,24 @@ void DomDistillerViewerSource::RequestViewerHandle::Cancel() {
   base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, this);
 }
 
-void DomDistillerViewerSource::RequestViewerHandle::DidFinishLoad(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url) {
+void DomDistillerViewerSource::RequestViewerHandle::DocumentLoadedInFrame(
+    content::RenderFrameHost* render_frame_host) {
+  // DocumentLoadedInFrame() is late enough to execute JavaScript, and is early
+  // enough so that it's more likely that the title and content can be picked up
+  // by TalkBack instead of the placeholder. If distillation is finished by
+  // DocumentLoadedInFrame(), onload() event would also be delayed, so that the
+  // accessibility focus is more likely to be on the web content. Otherwise, the
+  // focus is usually on the close button of the CustomTab (CCT), or nowhere. If
+  // distillation finishes later than DocumentLoadedInFrame(), or if for some
+  // reason the accessibility focus is on the close button of the CCT, the title
+  // could go unannounced.
+  // See http://crbug.com/811417.
   if (render_frame_host->GetParent()) {
     return;
   }
 
-  int64_t start_time_ms = url_utils::GetTimeFromDistillerUrl(validated_url);
+  int64_t start_time_ms = url_utils::GetTimeFromDistillerUrl(
+      render_frame_host->GetLastCommittedURL());
   if (start_time_ms > 0) {
     base::TimeTicks start_time =
         base::TimeDelta::FromMilliseconds(start_time_ms) + base::TimeTicks();