Commit 98257ea9 authored by Sam Bowen's avatar Sam Bowen Committed by Commit Bot

Reland "Rename CopylessPaste to DocumentMetadata to reuse the class for other purposes"

This is a two-sided patch and will break downstream clank until the following is landed:
https://chrome-internal-review.googlesource.com/c/clank/internal/apps/+/2473783

There are no changes to this reland. The fix for the original revert is
in the other patch linked above.

This is a reland of da0bf646

Original change's description:
> Rename CopylessPaste to DocumentMetadata to reuse the class for other purposes
>
> Bug: 1044244
> Change-Id: I7af8208b8c5a25f247b09cc0cbe55d44934f1d2d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2020604
> Reviewed-by: Becca Hughes <beccahughes@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: Sam Bowen <sgbowen@google.com>
> Cr-Commit-Position: refs/heads/master@{#735612}

TBR=dcheng@chromium.org,haraken@chromium.org

Bug: 1044244
Change-Id: I5348c86f79e39539d21956f82981a3e6d49d7009
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026409Reviewed-by: default avatarNatalie Chouinard <chouinard@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Commit-Queue: Sam Bowen <sgbowen@google.com>
Cr-Commit-Position: refs/heads/master@{#736439}
parent 4e30f923
......@@ -15,8 +15,8 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.base.Callback;
import org.chromium.base.SysUtils;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.blink.mojom.document_metadata.CopylessPaste;
import org.chromium.blink.mojom.document_metadata.WebPage;
import org.chromium.blink.mojom.DocumentMetadata;
import org.chromium.blink.mojom.WebPage;
import org.chromium.chrome.browser.historyreport.AppIndexingReporter;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModelSelector;
......@@ -59,7 +59,7 @@ public class AppIndexingUtil {
mObserver = new TabModelSelectorTabObserver(mTabModelSelectorImpl) {
@Override
public void onPageLoadFinished(final Tab tab, String url) {
extractCopylessPasteMetadata(tab);
extractDocumentMetadata(tab);
}
@Override
......@@ -80,7 +80,7 @@ public class AppIndexingUtil {
* title of the page is reported to App Indexing.
*/
@VisibleForTesting
void extractCopylessPasteMetadata(final Tab tab) {
void extractDocumentMetadata(final Tab tab) {
if (!isEnabledForTab(tab)) return;
// There are three conditions that can occur with respect to the cache.
......@@ -103,12 +103,12 @@ public class AppIndexingUtil {
// Condition 3
RecordHistogram.recordEnumeratedHistogram(
"CopylessPaste.CacheHit", CacheHit.MISS, CacheHit.NUM_ENTRIES);
CopylessPaste copylessPaste = getCopylessPasteInterface(tab);
if (copylessPaste == null) {
DocumentMetadata documentMetadata = getDocumentMetadataInterface(tab);
if (documentMetadata == null) {
return;
}
copylessPaste.getEntities(webpage -> {
copylessPaste.close();
documentMetadata.getEntities(webpage -> {
documentMetadata.close();
putCacheEntry(url, webpage != null);
if (sCallbackForTesting != null) {
sCallbackForTesting.onResult(webpage);
......@@ -165,7 +165,7 @@ public class AppIndexingUtil {
}
@VisibleForTesting
CopylessPaste getCopylessPasteInterface(Tab tab) {
DocumentMetadata getDocumentMetadataInterface(Tab tab) {
WebContents webContents = tab.getWebContents();
if (webContents == null) return null;
......@@ -175,7 +175,7 @@ public class AppIndexingUtil {
InterfaceProvider interfaces = mainFrame.getRemoteInterfaces();
if (interfaces == null) return null;
return interfaces.getInterface(CopylessPaste.MANAGER);
return interfaces.getInterface(DocumentMetadata.MANAGER);
}
@VisibleForTesting
......
......@@ -4,7 +4,7 @@
package org.chromium.chrome.browser.historyreport;
import org.chromium.blink.mojom.document_metadata.WebPage;
import org.chromium.blink.mojom.WebPage;
import org.chromium.chrome.browser.AppHooks;
/** Base class for reporting entities to App Indexing. */
......
......@@ -19,10 +19,10 @@ import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.Restriction;
import org.chromium.base.test.util.RetryOnFailure;
import org.chromium.blink.mojom.document_metadata.Entity;
import org.chromium.blink.mojom.document_metadata.Property;
import org.chromium.blink.mojom.document_metadata.Values;
import org.chromium.blink.mojom.document_metadata.WebPage;
import org.chromium.blink.mojom.Entity;
import org.chromium.blink.mojom.Property;
import org.chromium.blink.mojom.Values;
import org.chromium.blink.mojom.WebPage;
import org.chromium.chrome.browser.firstrun.FirstRunStatus;
import org.chromium.chrome.browser.util.UrlConstants;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
......
......@@ -24,8 +24,8 @@ import org.robolectric.annotation.Config;
import org.chromium.base.metrics.test.DisableHistogramsRule;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.blink.mojom.document_metadata.CopylessPaste;
import org.chromium.blink.mojom.document_metadata.WebPage;
import org.chromium.blink.mojom.DocumentMetadata;
import org.chromium.blink.mojom.WebPage;
import org.chromium.chrome.browser.historyreport.AppIndexingReporter;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.url.mojom.Url;
......@@ -43,7 +43,7 @@ public class AppIndexingUtilTest {
@Mock
private AppIndexingReporter mReporter;
@Mock
private CopylessPasteTestImpl mCopylessPaste;
private DocumentMetadataTestImpl mDocumentMetadata;
@Mock
private Tab mTab;
......@@ -51,7 +51,7 @@ public class AppIndexingUtilTest {
public void setUp() {
MockitoAnnotations.initMocks(this);
doReturn(mReporter).when(mUtil).getAppIndexingReporter();
doReturn(mCopylessPaste).when(mUtil).getCopylessPasteInterface(any(Tab.class));
doReturn(mDocumentMetadata).when(mUtil).getDocumentMetadataInterface(any(Tab.class));
doReturn(true).when(mUtil).isEnabledForDevice();
doReturn(false).when(mTab).isIncognito();
......@@ -59,67 +59,73 @@ public class AppIndexingUtilTest {
doReturn("My neat website").when(mTab).getTitle();
doReturn(0L).when(mUtil).getElapsedTime();
doAnswer(invocation -> {
CopylessPaste.GetEntitiesResponse callback =
(CopylessPaste.GetEntitiesResponse) invocation.getArguments()[0];
DocumentMetadata.GetEntitiesResponse callback =
(DocumentMetadata.GetEntitiesResponse) invocation.getArguments()[0];
WebPage webpage = new WebPage();
webpage.url = createUrl("http://www.test.com");
webpage.title = "My neat website";
callback.call(webpage);
return null;
}).when(mCopylessPaste).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
})
.when(mDocumentMetadata)
.getEntities(any(DocumentMetadata.GetEntitiesResponse.class));
}
@Test
public void testExtractCopylessPasteMetadata_Incognito() {
public void testExtractDocumentMetadata_Incognito() {
doReturn(true).when(mTab).isIncognito();
mUtil.extractCopylessPasteMetadata(mTab);
verify(mCopylessPaste, never()).getEntities(any());
mUtil.extractDocumentMetadata(mTab);
verify(mDocumentMetadata, never()).getEntities(any());
verify(mReporter, never()).reportWebPage(any());
}
@Test
public void testExtractCopylessPasteMetadata_NoCacheHit() {
mUtil.extractCopylessPasteMetadata(mTab);
verify(mCopylessPaste).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
public void testExtractDocumentMetadata_NoCacheHit() {
mUtil.extractDocumentMetadata(mTab);
verify(mDocumentMetadata).getEntities(any(DocumentMetadata.GetEntitiesResponse.class));
verify(mReporter).reportWebPage(any(WebPage.class));
}
@Test
public void testExtractCopylessPasteMetadata_CacheHit() {
mUtil.extractCopylessPasteMetadata(mTab);
verify(mCopylessPaste).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
verify(mCopylessPaste).close();
public void testExtractDocumentMetadata_CacheHit() {
mUtil.extractDocumentMetadata(mTab);
verify(mDocumentMetadata).getEntities(any(DocumentMetadata.GetEntitiesResponse.class));
verify(mDocumentMetadata).close();
verify(mReporter).reportWebPage(any(WebPage.class));
doReturn(1L).when(mUtil).getElapsedTime();
mUtil.extractCopylessPasteMetadata(mTab);
verifyNoMoreInteractions(mCopylessPaste);
mUtil.extractDocumentMetadata(mTab);
verifyNoMoreInteractions(mDocumentMetadata);
verifyNoMoreInteractions(mReporter);
}
@Test
public void testExtractCopylessPasteMetadata_CacheHit_Expired() {
mUtil.extractCopylessPasteMetadata(mTab);
public void testExtractDocumentMetadata_CacheHit_Expired() {
mUtil.extractDocumentMetadata(mTab);
doReturn(60 * 60 * 1000L + 1).when(mUtil).getElapsedTime();
mUtil.extractCopylessPasteMetadata(mTab);
verify(mCopylessPaste, times(2)).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
mUtil.extractDocumentMetadata(mTab);
verify(mDocumentMetadata, times(2))
.getEntities(any(DocumentMetadata.GetEntitiesResponse.class));
}
@Test
public void testExtractCopylessPasteMetadata_CacheHit_NoEntity() {
public void testExtractDocumentMetadata_CacheHit_NoEntity() {
doAnswer(invocation -> {
CopylessPaste.GetEntitiesResponse callback =
(CopylessPaste.GetEntitiesResponse) invocation.getArguments()[0];
DocumentMetadata.GetEntitiesResponse callback =
(DocumentMetadata.GetEntitiesResponse) invocation.getArguments()[0];
callback.call(null);
return null;
}).when(mCopylessPaste).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
mUtil.extractCopylessPasteMetadata(mTab);
})
.when(mDocumentMetadata)
.getEntities(any(DocumentMetadata.GetEntitiesResponse.class));
mUtil.extractDocumentMetadata(mTab);
doReturn(1L).when(mUtil).getElapsedTime();
mUtil.extractCopylessPasteMetadata(mTab);
verify(mCopylessPaste, times(1)).getEntities(any(CopylessPaste.GetEntitiesResponse.class));
mUtil.extractDocumentMetadata(mTab);
verify(mDocumentMetadata, times(1))
.getEntities(any(DocumentMetadata.GetEntitiesResponse.class));
verifyNoMoreInteractions(mReporter);
}
......@@ -143,5 +149,5 @@ public class AppIndexingUtilTest {
return url;
}
abstract static class CopylessPasteTestImpl implements CopylessPaste {}
abstract static class DocumentMetadataTestImpl implements DocumentMetadata {}
}
......@@ -233,7 +233,7 @@ mojom("android_mojo_bindings") {
"blob/blob_url_store.mojom",
"blob/data_element.mojom",
"blob/serialized_blob.mojom",
"document_metadata/copyless_paste.mojom",
"document_metadata/document_metadata.mojom",
"font_unique_name_lookup/font_unique_name_lookup.mojom",
"input/input_host.mojom",
"input/input_messages.mojom",
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
module blink.mojom.document_metadata;
module blink.mojom;
import "url/mojom/url.mojom";
......@@ -36,6 +36,6 @@ struct WebPage {
};
// Null page denotes no results.
interface CopylessPaste {
interface DocumentMetadata {
GetEntities() => (WebPage? page);
};
......@@ -281,7 +281,7 @@ jumbo_source_set("unit_tests") {
"csspaint/paint_worklet_test.cc",
"device_orientation/device_motion_event_pump_unittest.cc",
"device_orientation/device_orientation_event_pump_unittest.cc",
"document_metadata/copyless_paste_extractor_test.cc",
"document_metadata/document_metadata_extractor_test.cc",
"eventsource/event_source_parser_test.cc",
"filesystem/dom_file_system_base_test.cc",
"filesystem/file_writer_test.cc",
......
# Copyright 2017 The Chromium Authors. All rights reserved.
# Copyright 2017 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.
......@@ -6,10 +6,10 @@ import("//third_party/blink/renderer/modules/modules.gni")
blink_modules_sources("document_metadata") {
sources = [
"copyless_paste_extractor.cc",
"copyless_paste_extractor.h",
"copyless_paste_server.cc",
"copyless_paste_server.h",
"document_metadata_extractor.cc",
"document_metadata_extractor.h",
"document_metadata_server.cc",
"document_metadata_server.h",
]
deps = []
......
beccahughes@chromium.org
steimel@chromium.org
sgbowen@google.com
# COMPONENT: Blink>Media>Session
# TEAM: media-dev@chromium.org
......@@ -2,13 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/modules/document_metadata/copyless_paste_extractor.h"
#include "third_party/blink/renderer/modules/document_metadata/document_metadata_extractor.h"
#include <algorithm>
#include <memory>
#include <utility>
#include "third_party/blink/public/mojom/document_metadata/copyless_paste.mojom-blink.h"
#include "third_party/blink/public/mojom/document_metadata/document_metadata.mojom-blink.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/element_traversal.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
......@@ -26,14 +26,14 @@ namespace blink {
namespace {
using mojom::document_metadata::blink::Entity;
using mojom::document_metadata::blink::EntityPtr;
using mojom::document_metadata::blink::Property;
using mojom::document_metadata::blink::PropertyPtr;
using mojom::document_metadata::blink::Values;
using mojom::document_metadata::blink::ValuesPtr;
using mojom::document_metadata::blink::WebPage;
using mojom::document_metadata::blink::WebPagePtr;
using mojom::blink::Entity;
using mojom::blink::EntityPtr;
using mojom::blink::Property;
using mojom::blink::PropertyPtr;
using mojom::blink::Values;
using mojom::blink::ValuesPtr;
using mojom::blink::WebPage;
using mojom::blink::WebPagePtr;
// App Indexing enforces a max nesting depth of 5. Our top level message
// corresponds to the WebPage, so this only leaves 4 more levels. We will parse
......@@ -284,8 +284,8 @@ ExtractionStatus ExtractMetadata(const Element& root,
} // namespace
WebPagePtr CopylessPasteExtractor::Extract(const Document& document) {
TRACE_EVENT0("blink", "CopylessPasteExtractor::Extract");
WebPagePtr DocumentMetadataExtractor::Extract(const Document& document) {
TRACE_EVENT0("blink", "DocumentMetadataExtractor::Extract");
if (!document.GetFrame() || !document.GetFrame()->IsMainFrame())
return nullptr;
......
......@@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_COPYLESS_PASTE_EXTRACTOR_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_COPYLESS_PASTE_EXTRACTOR_H_
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_DOCUMENT_METADATA_EXTRACTOR_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_DOCUMENT_METADATA_EXTRACTOR_H_
#include "third_party/blink/public/mojom/document_metadata/copyless_paste.mojom-blink-forward.h"
#include "third_party/blink/public/mojom/document_metadata/document_metadata.mojom-blink-forward.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
......@@ -13,16 +13,15 @@ namespace blink {
class Document;
// Extract structured metadata (currently schema.org in JSON-LD) for the
// Copyless Paste feature. The extraction must be done after the document
// has finished parsing.
class MODULES_EXPORT CopylessPasteExtractor final {
STATIC_ONLY(CopylessPasteExtractor);
// Extract structured metadata (currently schema.org in JSON-LD). The extraction
// must be done after the document has finished parsing.
class MODULES_EXPORT DocumentMetadataExtractor final {
STATIC_ONLY(DocumentMetadataExtractor);
public:
static mojom::document_metadata::blink::WebPagePtr Extract(const Document&);
static mojom::blink::WebPagePtr Extract(const Document&);
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_COPYLESS_PASTE_EXTRACTOR_H_
#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_DOCUMENT_METADATA_EXTRACTOR_H_
......@@ -2,38 +2,38 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/modules/document_metadata/copyless_paste_server.h"
#include "third_party/blink/renderer/modules/document_metadata/document_metadata_server.h"
#include <memory>
#include <utility>
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/modules/document_metadata/copyless_paste_extractor.h"
#include "third_party/blink/renderer/modules/document_metadata/document_metadata_extractor.h"
namespace blink {
CopylessPasteServer::CopylessPasteServer(LocalFrame& frame) : frame_(frame) {}
DocumentMetadataServer::DocumentMetadataServer(LocalFrame& frame)
: frame_(frame) {}
void CopylessPasteServer::BindMojoReceiver(
void DocumentMetadataServer::BindMojoReceiver(
LocalFrame* frame,
mojo::PendingReceiver<mojom::document_metadata::blink::CopylessPaste>
receiver) {
mojo::PendingReceiver<mojom::blink::DocumentMetadata> receiver) {
DCHECK(frame);
// TODO(wychen): remove BindMojoReceiver pattern, and make this a service
// associated with frame lifetime.
mojo::MakeSelfOwnedReceiver(std::make_unique<CopylessPasteServer>(*frame),
mojo::MakeSelfOwnedReceiver(std::make_unique<DocumentMetadataServer>(*frame),
std::move(receiver));
}
void CopylessPasteServer::GetEntities(GetEntitiesCallback callback) {
void DocumentMetadataServer::GetEntities(GetEntitiesCallback callback) {
if (!frame_ || !frame_->GetDocument()) {
std::move(callback).Run(nullptr);
return;
}
std::move(callback).Run(
CopylessPasteExtractor::Extract(*frame_->GetDocument()));
DocumentMetadataExtractor::Extract(*frame_->GetDocument()));
}
} // namespace blink
......@@ -2,11 +2,11 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_COPYLESS_PASTE_SERVER_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_COPYLESS_PASTE_SERVER_H_
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_DOCUMENT_METADATA_SERVER_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_DOCUMENT_METADATA_SERVER_H_
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "third_party/blink/public/mojom/document_metadata/copyless_paste.mojom-blink.h"
#include "third_party/blink/public/mojom/document_metadata/document_metadata.mojom-blink.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"
......@@ -15,14 +15,14 @@ namespace blink {
class LocalFrame;
// Mojo interface to return extracted metadata for AppIndexing.
class MODULES_EXPORT CopylessPasteServer final
: public mojom::document_metadata::blink::CopylessPaste {
class MODULES_EXPORT DocumentMetadataServer final
: public mojom::blink::DocumentMetadata {
public:
explicit CopylessPasteServer(LocalFrame&);
explicit DocumentMetadataServer(LocalFrame&);
static void BindMojoReceiver(
LocalFrame*,
mojo::PendingReceiver<mojom::document_metadata::blink::CopylessPaste>);
mojo::PendingReceiver<mojom::blink::DocumentMetadata>);
void GetEntities(GetEntitiesCallback) override;
......@@ -32,4 +32,4 @@ class MODULES_EXPORT CopylessPasteServer final
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_COPYLESS_PASTE_SERVER_H_
#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_DOCUMENT_METADATA_DOCUMENT_METADATA_SERVER_H_
......@@ -44,7 +44,7 @@
#include "third_party/blink/renderer/modules/device_orientation/device_orientation_absolute_controller.h"
#include "third_party/blink/renderer/modules/device_orientation/device_orientation_controller.h"
#include "third_party/blink/renderer/modules/device_orientation/device_orientation_inspector_agent.h"
#include "third_party/blink/renderer/modules/document_metadata/copyless_paste_server.h"
#include "third_party/blink/renderer/modules/document_metadata/document_metadata_server.h"
#include "third_party/blink/renderer/modules/encryptedmedia/html_media_element_encrypted_media.h"
#include "third_party/blink/renderer/modules/encryptedmedia/media_keys_controller.h"
#include "third_party/blink/renderer/modules/event_interface_modules_names.h"
......@@ -160,7 +160,7 @@ void ModulesInitializer::Initialize() {
void ModulesInitializer::InitLocalFrame(LocalFrame& frame) const {
if (frame.IsMainFrame()) {
frame.GetInterfaceRegistry()->AddInterface(WTF::BindRepeating(
&CopylessPasteServer::BindMojoReceiver, WrapWeakPersistent(&frame)));
&DocumentMetadataServer::BindMojoReceiver, WrapWeakPersistent(&frame)));
}
if (RuntimeEnabledFeatures::FileHandlingEnabled(frame.GetDocument())) {
frame.GetInterfaceRegistry()->AddAssociatedInterface(WTF::BindRepeating(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment