From b7c113fdd42bce4317b197c470a9718e20aab895 Mon Sep 17 00:00:00 2001 From: Edgar Chen Date: Mon, 6 Jan 2025 15:53:05 +0000 Subject: [PATCH] Bug 1935127 - Part 1: Throw `NS_ERROR_NOT_AVAILABLE` error when the `ClipboardDataSnapshot` is no longer valid; r=nika Differential Revision: https://phabricator.services.mozilla.com/D231809 --- widget/ClipboardReadRequestParent.cpp | 4 +- widget/nsBaseClipboard.cpp | 6 +- widget/nsClipboardProxy.cpp | 17 +++--- widget/tests/clipboard_helper.js | 17 +++--- .../file_test_clipboard_getDataSnapshot.js | 57 +++++++++++++------ ...file_test_clipboard_getDataSnapshotSync.js | 46 +++++++++------ 6 files changed, 93 insertions(+), 54 deletions(-) diff --git a/widget/ClipboardReadRequestParent.cpp b/widget/ClipboardReadRequestParent.cpp index 70072cf5ce81..0aabfcd4d35c 100644 --- a/widget/ClipboardReadRequestParent.cpp +++ b/widget/ClipboardReadRequestParent.cpp @@ -74,7 +74,7 @@ IPCResult ClipboardReadRequestParent::RecvGetData( bool valid = false; if (NS_FAILED(mClipboardDataSnapshot->GetValid(&valid)) || !valid) { Unused << PClipboardReadRequestParent::Send__delete__(this); - aResolver(NS_ERROR_FAILURE); + aResolver(NS_ERROR_NOT_AVAILABLE); return IPC_OK(); } @@ -130,7 +130,7 @@ IPCResult ClipboardReadRequestParent::RecvGetDataSync( bool valid = false; if (NS_FAILED(mClipboardDataSnapshot->GetValid(&valid)) || !valid) { destroySoon(); - *aTransferableDataOrError = NS_ERROR_FAILURE; + *aTransferableDataOrError = NS_ERROR_NOT_AVAILABLE; return IPC_OK(); } diff --git a/widget/nsBaseClipboard.cpp b/widget/nsBaseClipboard.cpp index 2022562e3b6b..0c5485558f1a 100644 --- a/widget/nsBaseClipboard.cpp +++ b/widget/nsBaseClipboard.cpp @@ -1010,7 +1010,7 @@ NS_IMETHODIMP nsBaseClipboard::ClipboardDataSnapshot::GetData( } if (!IsValid()) { - aCallback->OnComplete(NS_ERROR_FAILURE); + aCallback->OnComplete(NS_ERROR_NOT_AVAILABLE); return NS_OK; } @@ -1066,7 +1066,7 @@ NS_IMETHODIMP nsBaseClipboard::ClipboardDataSnapshot::GetData( // `IsValid()` checks the clipboard sequence number to ensure the data // we are requesting is still valid. if (!self->IsValid()) { - callback->OnComplete(NS_ERROR_FAILURE); + callback->OnComplete(NS_ERROR_NOT_AVAILABLE); return; } mozilla::contentanalysis::ContentAnalysis:: @@ -1102,7 +1102,7 @@ NS_IMETHODIMP nsBaseClipboard::ClipboardDataSnapshot::GetDataSync( } if (!IsValid()) { - return NS_ERROR_FAILURE; + return NS_ERROR_NOT_AVAILABLE; } MOZ_ASSERT(mClipboard); diff --git a/widget/nsClipboardProxy.cpp b/widget/nsClipboardProxy.cpp index 99899fc52546..7e701e6b035b 100644 --- a/widget/nsClipboardProxy.cpp +++ b/widget/nsClipboardProxy.cpp @@ -27,6 +27,7 @@ #include "nsContentUtils.h" #include "PermissionMessageUtils.h" +using mozilla::ipc::ResponseRejectReason; using namespace mozilla; using namespace mozilla::dom; @@ -168,7 +169,7 @@ NS_IMETHODIMP ClipboardDataSnapshotProxy::GetData( } if (!mActor->CanSend()) { - return aCallback->OnComplete(NS_ERROR_FAILURE); + return aCallback->OnComplete(NS_ERROR_NOT_AVAILABLE); } mActor->SendGetData(flavors)->Then( @@ -196,9 +197,10 @@ NS_IMETHODIMP ClipboardDataSnapshotProxy::GetData( callback->OnComplete(NS_OK); }, /* reject */ - [callback = - nsCOMPtr{aCallback}](mozilla::ipc::ResponseRejectReason aReason) { - callback->OnComplete(NS_ERROR_FAILURE); + [callback = nsCOMPtr{aCallback}](ResponseRejectReason aReason) { + callback->OnComplete(ResponseRejectReason::ActorDestroyed == aReason + ? NS_ERROR_NOT_AVAILABLE + : NS_ERROR_FAILURE); }); return NS_OK; @@ -226,13 +228,13 @@ NS_IMETHODIMP ClipboardDataSnapshotProxy::GetDataSync( } if (!mActor->CanSend()) { - return NS_ERROR_FAILURE; + return NS_ERROR_NOT_AVAILABLE; } IPCTransferableDataOrError ipcTransferableDataOrError; bool success = mActor->SendGetDataSync(flavors, &ipcTransferableDataOrError); if (!success) { - return NS_ERROR_FAILURE; + return NS_ERROR_NOT_AVAILABLE; } if (ipcTransferableDataOrError.type() == IPCTransferableDataOrError::Tnsresult) { @@ -309,8 +311,7 @@ NS_IMETHODIMP nsClipboardProxy::GetDataSnapshot( callback->OnSuccess(result.inspect()); }, /* reject */ - [callback = nsCOMPtr{aCallback}]( - mozilla::ipc::ResponseRejectReason aReason) { + [callback = nsCOMPtr{aCallback}](ResponseRejectReason aReason) { callback->OnError(NS_ERROR_FAILURE); }); return NS_OK; diff --git a/widget/tests/clipboard_helper.js b/widget/tests/clipboard_helper.js index 18aa06216713..86ec3df429bb 100644 --- a/widget/tests/clipboard_helper.js +++ b/widget/tests/clipboard_helper.js @@ -234,18 +234,18 @@ function asyncClipboardRequestGetData(aRequest, aFlavor, aThrows = false) { aThrows ? "throw" : "success" }` ); - reject(e); + reject(e.result); } }); } -function syncClipboardRequestGetData(aRequest, aFlavor, aThrows = false) { +function syncClipboardRequestGetData(aRequest, aFlavor, aResult = Cr.NS_OK) { var trans = Cc["@mozilla.org/widget/transferable;1"].createInstance( Ci.nsITransferable ); trans.init(null); trans.addDataFlavor(aFlavor); - let error = undefined; + let result = Cr.NS_OK; try { aRequest.getDataSync(trans); try { @@ -258,12 +258,13 @@ function syncClipboardRequestGetData(aRequest, aFlavor, aThrows = false) { return ""; } } catch (e) { - error = e; - return error; + result = e.result; } finally { - ok( - aThrows === (error !== undefined), - `nsIAsyncGetClipboardData.getData should ${aThrows ? "throw" : "success"}` + is( + result, + aResult, + `nsIAsyncGetClipboardData.getData should ${aResult == Cr.NS_OK ? "throw" : "success"}` ); } + return ""; } diff --git a/widget/tests/file_test_clipboard_getDataSnapshot.js b/widget/tests/file_test_clipboard_getDataSnapshot.js index 5da02ed36348..dea4804b1e48 100644 --- a/widget/tests/file_test_clipboard_getDataSnapshot.js +++ b/widget/tests/file_test_clipboard_getDataSnapshot.js @@ -65,10 +65,14 @@ clipboardTypes.forEach(function (type) { let request = await getClipboardDataSnapshot(type); isDeeply(request.flavorList, [], "Check flavorList"); - await asyncClipboardRequestGetData(request, "text/plain", true).catch( - () => {} - ); - syncClipboardRequestGetData(request, "text/plain", true); + await asyncClipboardRequestGetData(request, "text/plain", true).catch(e => { + is( + e, + Cr.NS_ERROR_FAILURE, + "should be rejected with NS_ERROR_FAILURE error" + ); + }); + syncClipboardRequestGetData(request, "text/plain", Cr.NS_ERROR_FAILURE); }); add_task(async function test_clipboard_getDataSnapshot_after_write() { @@ -90,11 +94,15 @@ clipboardTypes.forEach(function (type) { ); ok(request.valid, "request should still be valid"); // Requesting a flavor that is not in the list should throw error. - await asyncClipboardRequestGetData(request, "text/html", true).catch( - () => {} - ); + await asyncClipboardRequestGetData(request, "text/html", true).catch(e => { + is( + e, + Cr.NS_ERROR_FAILURE, + "should be rejected with NS_ERROR_FAILURE error" + ); + }); ok(request.valid, "request should still be valid"); - syncClipboardRequestGetData(request, "text/html", true); + syncClipboardRequestGetData(request, "text/html", Cr.NS_ERROR_FAILURE); ok(request.valid, "request should still be valid"); // Writing a new data should invalid existing get request. @@ -103,12 +111,21 @@ clipboardTypes.forEach(function (type) { () => { ok(false, "asyncClipboardRequestGetData should not success"); }, - () => { + e => { + is( + e, + Cr.NS_ERROR_NOT_AVAILABLE, + "should throw NS_ERROR_NOT_AVAILABLE error" + ); ok(true, "asyncClipboardRequestGetData should reject"); } ); ok(!request.valid, "request should no longer be valid"); - syncClipboardRequestGetData(request, "text/plain", true); + syncClipboardRequestGetData( + request, + "text/plain", + Cr.NS_ERROR_NOT_AVAILABLE + ); ok(!request.valid, "request should no longer be valid"); info(`check clipboard data again`); @@ -155,8 +172,12 @@ clipboardTypes.forEach(function (type) { () => { ok(false, "asyncClipboardRequestGetData should not success"); }, - () => { - ok(true, "asyncClipboardRequestGetData should reject"); + e => { + is( + e, + Cr.NS_ERROR_NOT_AVAILABLE, + "asyncClipboardRequestGetData should reject with NS_ERROR_NOT_AVAILABLE error" + ); } ); ok(!request.valid, "request should no longer be valid"); @@ -187,9 +208,13 @@ add_task(async function test_html_data() { "Check data" ); // Requesting a flavor that is not in the list should throw error. - await asyncClipboardRequestGetData(request, "text/plain", true).catch( - () => {} - ); + await asyncClipboardRequestGetData(request, "text/plain", true).catch(e => { + is( + e, + Cr.NS_ERROR_FAILURE, + "should be rejected with NS_ERROR_FAILURE error" + ); + }); is( syncClipboardRequestGetData(request, "text/html"), @@ -197,5 +222,5 @@ add_task(async function test_html_data() { "Check data (sync)" ); // Requesting a flavor that is not in the list should throw error. - syncClipboardRequestGetData(request, "text/plain", true); + syncClipboardRequestGetData(request, "text/plain", Cr.NS_ERROR_FAILURE); }); diff --git a/widget/tests/file_test_clipboard_getDataSnapshotSync.js b/widget/tests/file_test_clipboard_getDataSnapshotSync.js index ac1e48724626..07717490fe1d 100644 --- a/widget/tests/file_test_clipboard_getDataSnapshotSync.js +++ b/widget/tests/file_test_clipboard_getDataSnapshotSync.js @@ -48,10 +48,10 @@ clipboardTypes.forEach(function (type) { let request = getClipboardDataSnapshotSync(type); isDeeply(request.flavorList, [], "Check flavorList"); - await asyncClipboardRequestGetData(request, "text/plain", true).catch( - () => {} - ); - syncClipboardRequestGetData(request, "text/plain", true); + await asyncClipboardRequestGetData(request, "text/plain", true).catch(e => { + is(e, Cr.NS_ERROR_FAILURE, "should throw NS_ERROR_FAILURE error"); + }); + syncClipboardRequestGetData(request, "text/plain", Cr.NS_ERROR_FAILURE); }); add_task(async function test_clipboard_getDataSnapshotSync_after_write() { @@ -73,11 +73,11 @@ clipboardTypes.forEach(function (type) { ); ok(request.valid, "request should still be valid"); // Requesting a flavor that is not in the list should throw error. - await asyncClipboardRequestGetData(request, "text/html", true).catch( - () => {} - ); + await asyncClipboardRequestGetData(request, "text/html", true).catch(e => { + is(e, Cr.NS_ERROR_FAILURE, "should throw NS_ERROR_FAILURE error"); + }); ok(request.valid, "request should still be valid"); - syncClipboardRequestGetData(request, "text/html", true); + syncClipboardRequestGetData(request, "text/html", Cr.NS_ERROR_FAILURE); ok(request.valid, "request should still be valid"); // Writing a new data should invalid existing get request. @@ -86,12 +86,20 @@ clipboardTypes.forEach(function (type) { () => { ok(false, "asyncClipboardRequestGetData should not success"); }, - () => { - ok(true, "asyncClipboardRequestGetData should reject"); + e => { + is( + e, + Cr.NS_ERROR_NOT_AVAILABLE, + "should throw NS_ERROR_NOT_AVAILABLE error" + ); } ); ok(!request.valid, "request should no longer be valid"); - syncClipboardRequestGetData(request, "text/plain", true); + syncClipboardRequestGetData( + request, + "text/plain", + Cr.NS_ERROR_NOT_AVAILABLE + ); ok(!request.valid, "request should no longer be valid"); info(`check clipboard data again`); @@ -132,8 +140,12 @@ clipboardTypes.forEach(function (type) { () => { ok(false, "asyncClipboardRequestGetData should not success"); }, - () => { - ok(true, "asyncClipboardRequestGetData should reject"); + e => { + is( + e, + Cr.NS_ERROR_NOT_AVAILABLE, + "should throw NS_ERROR_NOT_AVAILABLE error" + ); } ); ok(!request.valid, "request should no longer be valid"); @@ -164,9 +176,9 @@ add_task(async function test_clipboard_getDataSnapshotSync_html_data() { "Check data" ); // Requesting a flavor that is not in the list should throw error. - await asyncClipboardRequestGetData(request, "text/plain", true).catch( - () => {} - ); + await asyncClipboardRequestGetData(request, "text/plain", true).catch(e => { + is(e, Cr.NS_ERROR_FAILURE, "should throw NS_ERROR_FAILURE error"); + }); is( syncClipboardRequestGetData(request, "text/html"), @@ -174,5 +186,5 @@ add_task(async function test_clipboard_getDataSnapshotSync_html_data() { "Check data (sync)" ); // Requesting a flavor that is not in the list should throw error. - syncClipboardRequestGetData(request, "text/plain", true); + syncClipboardRequestGetData(request, "text/plain", Cr.NS_ERROR_FAILURE); });