sharing: show high-quality preview images in share hub

This change: 1. Defines a new IPC, blink.mojom.LocalFrame.GetOpenGraphMetadata, which fetches the frame's OpenGraph metadata, and an associated type blink.mojom.OpenGraphMetadata[1]. 2. Implements that IPC on renderer and browser sides. 3. Defines a new network traffic annotation share_preview_image_fetch for the actual image fetching for the sharing hub preview. 4. Adds code to SharingHubBubbleController to fetch the OpenGraph metadata for the current main frame, extract the image URL from it, fetch and decode that image safely using the data decoding service, and plug it into the sharing hub. Open questions for review: * What is the best way to test the new IPC? Bug: 1314854 [1]: That type contains only the image URL, but can be easily extended to contain other OpenGraph metadata if other clients need it. Change-Id: Ib5f0695461416e5d80e10612926df55e1314d292 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3597083 Reviewed-by: Nicolas Ouellet-Payeur <nicolaso@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Kristi Park <kristipark@chromium.org> Reviewed-by: Stephen Chenney <schenney@chromium.org> Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/main@{#1001536}
chromium · May 10, 2022 · dae8f5c · dae8f5c
1 parent d8430a7
commit dae8f5c
Show file tree

Hide file tree

Showing 21 changed files with 224 additions and 1 deletion.
diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn
@@ -5048,6 +5048,7 @@ static_library("ui") {
       "//components/user_notes/browser",
       "//components/user_notes/interfaces",
       "//device/vr/buildflags:buildflags",
+      "//services/data_decoder/public/cpp",
       "//services/media_session/public/mojom",
       "//ui/base/dragdrop:types",
       "//ui/gfx/geometry",

diff --git a/chrome/browser/ui/sharing_hub/sharing_hub_bubble_controller_desktop_impl.cc b/chrome/browser/ui/sharing_hub/sharing_hub_bubble_controller_desktop_impl.cc
@@ -8,6 +8,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/favicon/favicon_utils.h"
+#include "chrome/browser/image_fetcher/image_decoder_impl.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/share/share_features.h"
 #include "chrome/browser/share/share_metrics.h"
@@ -23,11 +24,55 @@
 #include "chrome/browser/ui/send_tab_to_self/send_tab_to_self_bubble_controller.h"
 #include "chrome/browser/ui/sharing_hub/sharing_hub_bubble_view.h"
 #include "chrome/grit/generated_resources.h"
+#include "components/image_fetcher/core/image_fetcher.h"
+#include "components/image_fetcher/core/image_fetcher_impl.h"
+#include "content/public/browser/site_instance.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
+#include "third_party/blink/public/mojom/opengraph/metadata.mojom.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace sharing_hub {
 
+namespace {
+
+constexpr char kPreviewUmaClient[] = "SharePreview";
+
+constexpr net::NetworkTrafficAnnotationTag kPreviewImageNetworkAnnotationTag =
+    net::DefineNetworkTrafficAnnotation("share_preview_image_fetch",
+                                        R"(
+      semantics {
+        sender: "Share bubble"
+        description:
+            "The share bubble offers a preview of the site or image being "
+            "shared. For sites, this image is specified by the site author "
+            "using OpenGraph metadata. If this metadata is present on the "
+            "site being shared and specifies a preview image, the share "
+            "bubble fetches the image to display it."
+        trigger:
+            "User presses 'Share' on a page that has OpenGraph metadata."
+        data:
+            "The image URL being requested from the site. Since the user has "
+            "already visited the site to trigger the sharing flow, this "
+            "request is similar to a request for any other part of the page."
+        destination: WEBSITE
+      }
+      policy {
+        cookies_allowed: NO
+        setting:
+          "Administrators can disable this feature by disabling the share "
+          "bubble altogether, which can be done via policy. There is no "
+          "specific way to disable loading the preview image."
+        chrome_policy: {
+          DesktopSharingHubEnabled: {
+            DesktopSharingHubEnabled: false
+          }
+        }
+      }
+  )");
+
+}  // namespace
+
 // static
 // SharingHubBubbleController:
 SharingHubBubbleController*
@@ -59,6 +104,9 @@ void SharingHubBubbleControllerDesktopImpl::ShowBubble() {
   sharing_hub_bubble_view_ =
       browser->window()->ShowSharingHubBubble(web_contents());
 
+  if (ShouldUsePreview())
+    FetchHQImageForPreview();
+
   share::LogShareSourceDesktop(share::ShareSourceDesktop::kOmniboxSharingHub);
 }
 
@@ -178,6 +226,43 @@ SharingHubModel* SharingHubBubbleControllerDesktopImpl::GetSharingHubModel() {
   return sharing_hub_model_;
 }
 
+void SharingHubBubbleControllerDesktopImpl::FetchHQImageForPreview() {
+  content::RenderFrameHost& main_frame =
+      GetWebContents().GetPrimaryPage().GetMainDocument();
+  main_frame.GetOpenGraphMetadata(base::BindOnce(
+      &SharingHubBubbleControllerDesktopImpl::OnGetOpenGraphMetadata,
+      AsWeakPtr()));
+}
+
+void SharingHubBubbleControllerDesktopImpl::OnGetOpenGraphMetadata(
+    blink::mojom::OpenGraphMetadataPtr metadata) {
+  if (!metadata->image)
+    return;
+
+  auto* profile =
+      Profile::FromBrowserContext(GetWebContents().GetBrowserContext());
+  if (!profile)
+    return;
+
+  image_fetcher_ = std::make_unique<image_fetcher::ImageFetcherImpl>(
+      std::make_unique<ImageDecoderImpl>(),
+      profile->GetDefaultStoragePartition()
+          ->GetURLLoaderFactoryForBrowserProcess());
+
+  image_fetcher_->FetchImage(
+      *metadata->image,
+      base::BindOnce(&SharingHubBubbleControllerDesktopImpl::OnGetHQImage,
+                     AsWeakPtr()),
+      image_fetcher::ImageFetcherParams(kPreviewImageNetworkAnnotationTag,
+                                        kPreviewUmaClient));
+}
+
+void SharingHubBubbleControllerDesktopImpl::OnGetHQImage(
+    const gfx::Image& image,
+    const image_fetcher::RequestMetadata& metadata) {
+  preview_image_changed_callbacks_.Notify(ui::ImageModel::FromImage(image));
+}
+
 SharingHubBubbleControllerDesktopImpl::SharingHubBubbleControllerDesktopImpl(
     content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),

diff --git a/chrome/browser/ui/sharing_hub/sharing_hub_bubble_controller_desktop_impl.h b/chrome/browser/ui/sharing_hub/sharing_hub_bubble_controller_desktop_impl.h
@@ -12,6 +12,7 @@
 #include "chrome/browser/ui/sharing_hub/sharing_hub_bubble_controller.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
+#include "third_party/blink/public/mojom/opengraph/metadata.mojom-forward.h"
 #include "ui/base/models/image_model.h"
 
 class Profile;
@@ -20,6 +21,11 @@ namespace content {
 class WebContents;
 }  // namespace content
 
+namespace image_fetcher {
+class ImageFetcher;
+struct RequestMetadata;
+}  // namespace image_fetcher
+
 namespace sharing_hub {
 
 class SharingHubBubbleView;
@@ -88,6 +94,16 @@ class SharingHubBubbleControllerDesktopImpl
 
   SharingHubModel* GetSharingHubModel();
 
+  // These three methods handle fetching and displaying high-quality preview
+  // images. The first starts the process of fetching the page's OpenGraph
+  // metadata. The second receives the resulting metadata and issues a request
+  // to fetch and decode the referenced image. The third takes the received HQ
+  // preview image and passes it to the preview view for display.
+  void FetchHQImageForPreview();
+  void OnGetOpenGraphMetadata(blink::mojom::OpenGraphMetadataPtr metadata);
+  void OnGetHQImage(const gfx::Image& image,
+                    const image_fetcher::RequestMetadata&);
+
   // Weak reference. Will be nullptr if no bubble is currently shown.
   raw_ptr<SharingHubBubbleView> sharing_hub_bubble_view_ = nullptr;
   // Cached reference to the model.
@@ -96,6 +112,8 @@ class SharingHubBubbleControllerDesktopImpl
   base::RepeatingCallbackList<void(ui::ImageModel)>
       preview_image_changed_callbacks_;
 
+  std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher_;
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 };
 

diff --git a/chrome/browser/ui/views/sharing_hub/preview_view.cc b/chrome/browser/ui/views/sharing_hub/preview_view.cc
@@ -75,6 +75,7 @@ void PreviewView::TakeCallbackSubscription(
 
 void PreviewView::OnImageChanged(ui::ImageModel model) {
   image_->SetImage(model);
+  image_->SetImageSize(kImageSize);
 }
 
 }  // namespace sharing_hub
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -241,6 +241,7 @@
 #include "third_party/blink/public/mojom/loader/resource_load_info.mojom.h"
 #include "third_party/blink/public/mojom/loader/transferrable_url_loader.mojom.h"
 #include "third_party/blink/public/mojom/navigation/navigation_params.mojom.h"
+#include "third_party/blink/public/mojom/opengraph/metadata.mojom.h"
 #include "third_party/blink/public/mojom/page/display_cutout.mojom.h"
 #include "third_party/blink/public/mojom/scroll/scroll_into_view_params.mojom.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_object.mojom.h"
@@ -1072,6 +1073,19 @@ bool IsWindowPlacementGranted(RenderFrameHost* host) {
          blink::mojom::PermissionStatus::GRANTED;
 }
 
+bool IsOpenGraphMetadataValid(const blink::mojom::OpenGraphMetadata* metadata) {
+  return !metadata->image || metadata->image->SchemeIsHTTPOrHTTPS();
+}
+
+void ForwardOpenGraphMetadataIfValid(
+    base::OnceCallback<void(blink::mojom::OpenGraphMetadataPtr)> callback,
+    blink::mojom::OpenGraphMetadataPtr metadata) {
+  if (IsOpenGraphMetadataValid(metadata.get()))
+    std::move(callback).Run(std::move(metadata));
+  else
+    std::move(callback).Run({});
+}
+
 }  // namespace
 
 class RenderFrameHostImpl::SubresourceLoaderFactoriesConfig {
@@ -2199,6 +2213,16 @@ void RenderFrameHostImpl::GetCanonicalUrl(
   }
 }
 
+void RenderFrameHostImpl::GetOpenGraphMetadata(
+    base::OnceCallback<void(blink::mojom::OpenGraphMetadataPtr)> callback) {
+  if (IsRenderFrameCreated()) {
+    GetAssociatedLocalFrame()->GetOpenGraphMetadata(
+        base::BindOnce(&ForwardOpenGraphMetadataIfValid, std::move(callback)));
+  } else {
+    std::move(callback).Run({});
+  }
+}
+
 bool RenderFrameHostImpl::IsErrorDocument() {
   // This shouldn't be called before committing the document as this value is
   // set during call to RenderFrameHostImpl::DidNavigate which happens after

diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h
@@ -448,6 +448,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
   void WriteIntoTrace(perfetto::TracedProto<TraceProto> context) const override;
   void GetCanonicalUrl(
       base::OnceCallback<void(const absl::optional<GURL>&)> callback) override;
+  void GetOpenGraphMetadata(
+      base::OnceCallback<void(blink::mojom::OpenGraphMetadataPtr)> callback)
+      override;
   bool IsErrorDocument() override;
   DocumentRef GetDocumentRef() override;
   WeakDocumentPtr GetWeakDocumentPtr() override;

diff --git a/content/public/browser/render_frame_host.h b/content/public/browser/render_frame_host.h
@@ -30,6 +30,7 @@
 #include "third_party/blink/public/mojom/frame/frame.mojom-forward.h"
 #include "third_party/blink/public/mojom/frame/sudden_termination_disabler_type.mojom-forward.h"
 #include "third_party/blink/public/mojom/frame/user_activation_notification_type.mojom-forward.h"
+#include "third_party/blink/public/mojom/opengraph/metadata.mojom-forward.h"
 #include "third_party/blink/public/mojom/page/page_visibility_state.mojom-forward.h"
 #include "third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom-forward.h"
 #include "third_party/perfetto/include/perfetto/tracing/traced_value_forward.h"
@@ -1014,6 +1015,13 @@ class CONTENT_EXPORT RenderFrameHost : public IPC::Listener,
   virtual void GetCanonicalUrl(
       base::OnceCallback<void(const absl::optional<GURL>&)> callback) = 0;
 
+  // Fetch the OpenGraph metadata from the renderer process. The returned data
+  // has only been validated as follows:
+  // * Contained URLs are web schemes, not other schemes
+  // Any other properties you want, you'll need to check yourself.
+  virtual void GetOpenGraphMetadata(
+      base::OnceCallback<void(blink::mojom::OpenGraphMetadataPtr)>) = 0;
+
   // Returns true if the last navigation in this RenderFrameHost has committed
   // an error document that is a placeholder document installed when the
   // navigation failed or was blocked, containing an error message like "This

diff --git a/content/public/test/fake_local_frame.cc b/content/public/test/fake_local_frame.cc
@@ -188,6 +188,9 @@ void FakeLocalFrame::HandleRendererDebugURL(const GURL& url) {}
 void FakeLocalFrame::GetCanonicalUrlForSharing(
     base::OnceCallback<void(const absl::optional<GURL>&)> callback) {}
 
+void FakeLocalFrame::GetOpenGraphMetadata(
+    base::OnceCallback<void(blink::mojom::OpenGraphMetadataPtr)>) {}
+
 void FakeLocalFrame::SetNavigationApiHistoryEntriesForRestore(
     blink::mojom::NavigationApiHistoryEntryArraysPtr entry_arrays) {}
 

diff --git a/content/public/test/fake_local_frame.h b/content/public/test/fake_local_frame.h
@@ -140,6 +140,8 @@ class FakeLocalFrame : public blink::mojom::LocalFrame {
   void HandleRendererDebugURL(const GURL& url) override;
   void GetCanonicalUrlForSharing(
       base::OnceCallback<void(const absl::optional<GURL>&)> callback) override;
+  void GetOpenGraphMetadata(
+      base::OnceCallback<void(blink::mojom::OpenGraphMetadataPtr)>) override;
   void SetNavigationApiHistoryEntriesForRestore(
       blink::mojom::NavigationApiHistoryEntryArraysPtr entry_arrays) override;
 

diff --git a/third_party/blink/public/mojom/BUILD.gn b/third_party/blink/public/mojom/BUILD.gn
@@ -142,6 +142,7 @@ mojom("mojom_platform") {
     "notifications/notification.mojom",
     "notifications/notification_service.mojom",
     "oom_intervention/oom_intervention.mojom",
+    "opengraph/metadata.mojom",
     "parakeet/ad_request.mojom",
     "payments/payment_app.mojom",
     "peerconnection/peer_connection_tracker.mojom",

diff --git a/third_party/blink/public/mojom/frame/frame.mojom b/third_party/blink/public/mojom/frame/frame.mojom
@@ -56,6 +56,7 @@ import "third_party/blink/public/mojom/input/scroll_direction.mojom";
 import "third_party/blink/public/mojom/navigation/navigation_api_history_entry_arrays.mojom";
 import "third_party/blink/public/mojom/navigation/navigation_policy.mojom";
 import "third_party/blink/public/mojom/loader/referrer.mojom";
+import "third_party/blink/public/mojom/opengraph/metadata.mojom";
 import "third_party/blink/public/mojom/widget/platform_widget.mojom";
 import "third_party/blink/public/mojom/page/widget.mojom";
 import "third_party/blink/public/mojom/portal/portal.mojom";
@@ -847,6 +848,11 @@ interface LocalFrame {
   // the document of this frame doesn't have the HTMLLinkElement.
   GetCanonicalUrlForSharing() => (url.mojom.Url? canonical_url);
 
+  // Requests the OpenGraph metadata for the page. See https://ogp.me/. If the
+  // page has no OpenGraph tags that are understood by this implementation, or
+  // if it has no OpenGraph tags at all, an empty OpenGraphMetadata is returned.
+  GetOpenGraphMetadata() => (blink.mojom.OpenGraphMetadata metadata);
+
   // Updates navigation.entries() when restoring from bfcache.
   // navigation.entries() represents a subset of the back/forward list visible
   // to this frame, and that subset may have changed while the page was in

diff --git a/third_party/blink/public/mojom/opengraph/OWNERS b/third_party/blink/public/mojom/opengraph/OWNERS
@@ -0,0 +1,2 @@
+per-file *.mojom=set noparent
+per-file *.mojom=file://ipc/SECURITY_OWNERS
diff --git a/third_party/blink/public/mojom/opengraph/metadata.mojom b/third_party/blink/public/mojom/opengraph/metadata.mojom
@@ -0,0 +1,23 @@
+// Copyright 2022 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.
+
+module blink.mojom;
+
+import "url/mojom/url.mojom";
+
+// An instance of OpenGraphMetadata represents the OpenGraph metadata extracted
+// from a page's meta tags - see https://ogp.me for more details. At the moment,
+// this is only a partial implementation to meet the needs of one use case.
+//
+// Security note: all this metadata comes directly from the page and while it
+// has been syntactically validated (by Mojo) no semantic checking has been
+// done. In particular, there is no guarantee that the various URLs, if present,
+// actually refer to images and in general they may be assumed to point to
+// arbitrary, untrusted web content.
+struct OpenGraphMetadata {
+  // The URL included in the og:image meta tag, or (as a compatibility behavior)
+  // the URL included in the image itemprop meta tag if that was present and no
+  // og:image is present.
+  url.mojom.Url? image;
+};
diff --git a/third_party/blink/renderer/core/frame/local_frame_mojo_handler.cc b/third_party/blink/renderer/core/frame/local_frame_mojo_handler.cc
@@ -17,6 +17,7 @@
 #include "third_party/blink/public/mojom/devtools/inspector_issue.mojom-blink.h"
 #include "third_party/blink/public/mojom/frame/frame_owner_properties.mojom-blink.h"
 #include "third_party/blink/public/mojom/frame/media_player_action.mojom-blink.h"
+#include "third_party/blink/public/mojom/opengraph/metadata.mojom-blink.h"
 #include "third_party/blink/public/platform/interface_registry.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/web/web_frame_serializer.h"
@@ -46,6 +47,7 @@
 #include "third_party/blink/renderer/core/fullscreen/fullscreen.h"
 #include "third_party/blink/renderer/core/html/html_element.h"
 #include "third_party/blink/renderer/core/html/html_link_element.h"
+#include "third_party/blink/renderer/core/html/html_meta_element.h"
 #include "third_party/blink/renderer/core/html/media/html_video_element.h"
 #include "third_party/blink/renderer/core/html/portal/dom_window_portal_host.h"
 #include "third_party/blink/renderer/core/html/portal/portal_activate_event.h"
@@ -320,6 +322,19 @@ HitTestResult HitTestResultForRootFramePos(
   return result;
 }
 
+void ParseOpenGraphProperty(const HTMLMetaElement& element,
+                            const Document& document,
+                            mojom::blink::OpenGraphMetadata* metadata) {
+  if (element.Property() == "og:image" && !metadata->image)
+    metadata->image = document.CompleteURL(element.Content());
+
+  // Non-OpenGraph, non-standard thing that some sites use the same way:
+  // using <meta itemprop="image" content="$url">, which means the same thing
+  // as <meta property="og:image" content="$url".
+  if (element.Itemprop() == "image" && !metadata->image)
+    metadata->image = document.CompleteURL(element.Content());
+}
+
 }  // namespace
 
 ActiveURLMessageFilter::~ActiveURLMessageFilter() {
@@ -1153,6 +1168,20 @@ void LocalFrameMojoHandler::GetCanonicalUrlForSharing(
 #endif
 }
 
+void LocalFrameMojoHandler::GetOpenGraphMetadata(
+    GetOpenGraphMetadataCallback callback) {
+  auto metadata = mojom::blink::OpenGraphMetadata::New();
+  for (const auto& child : Traversal<HTMLMetaElement>::DescendantsOf(
+           *frame_->GetDocument()->documentElement())) {
+    // If there are multiple OpenGraph tags for the same property, we always
+    // take the value from the first one - this is the specified behavior in
+    // the OpenGraph spec:
+    //   The first tag (from top to bottom) is given preference during conflicts
+    ParseOpenGraphProperty(child, *frame_->GetDocument(), metadata.get());
+  }
+  std::move(callback).Run(std::move(metadata));
+}
+
 void LocalFrameMojoHandler::SetNavigationApiHistoryEntriesForRestore(
     mojom::blink::NavigationApiHistoryEntryArraysPtr entry_arrays) {
   if (NavigationApi* navigation_api =

diff --git a/third_party/blink/renderer/core/frame/local_frame_mojo_handler.h b/third_party/blink/renderer/core/frame/local_frame_mojo_handler.h
@@ -191,6 +191,7 @@ class LocalFrameMojoHandler
   void HandleRendererDebugURL(const KURL& url) final;
   void GetCanonicalUrlForSharing(
       GetCanonicalUrlForSharingCallback callback) final;
+  void GetOpenGraphMetadata(GetOpenGraphMetadataCallback callback) final;
 
   void SetNavigationApiHistoryEntriesForRestore(
       mojom::blink::NavigationApiHistoryEntryArraysPtr) final;