Bug 202604 - Domain relationships in the ITP Database should be inserted in a single query and ignore repeat insert attempts.
Summary: Domain relationships in the ITP Database should be inserted in a single query...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Katherine_cheney
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-04 14:46 PDT by Katherine_cheney
Modified: 2019-10-07 16:17 PDT (History)
5 users (show)

See Also:


Attachments
Patch (106.65 KB, patch)
2019-10-04 16:14 PDT, Katherine_cheney
no flags Details | Formatted Diff | Diff
Patch (106.57 KB, patch)
2019-10-07 14:00 PDT, Katherine_cheney
no flags Details | Formatted Diff | Diff
Patch (106.56 KB, patch)
2019-10-07 15:56 PDT, Katherine_cheney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Katherine_cheney 2019-10-04 14:46:14 PDT
Domain relationships in the ITP Database should be inserted in a single query and ignore repeat insert attempts.
Comment 1 Radar WebKit Bug Importer 2019-10-04 14:46:26 PDT
<rdar://problem/55995831>
Comment 2 Katherine_cheney 2019-10-04 16:14:30 PDT
Created attachment 380264 [details]
Patch
Comment 3 Chris Dumez 2019-10-07 13:30:49 PDT
Comment on attachment 380264 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=380264&action=review

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1738
> +bool ResourceLoadStatisticsDatabaseStore::countSubStatisticsTesting(const RegistrableDomain& subframeDomain, const TopFrameDomain& topFrameDomain)

Please find a better name, this is called "count" and yet returns a boolean instead of an integer. This is confusing.

> Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp:1768
> +    if (subFrameUnderTopFrameCount.getColumnInt(0) == 1

return (subFrameUnderTopFrameCount.getColumnInt(0) == 1
    && subresourceUnderTopFrameCount.getColumnInt(0) == 1
    && subresourceUniqueRedirectsTo.getColumnInt(0) == 1);

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:638
> +            return;

You're failing to call the completion handler here.

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:640
> +        bool isRelationshipOnlyInDatabaseOnce = downcast<ResourceLoadStatisticsDatabaseStore>(*m_statisticsStore.get()).countSubStatisticsTesting(subDomain, topDomain);

You should not need the .get() I think.

> Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:606
> +    if (!canSendMessage()) {

Not needed, sendWithAsyncReply() should do the right thing already.

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:1750
> +    WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("IsStatisticsOnlyInDatabaseOnce"));

auto

> Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:1751
> +    WKRetainPtr<WKDictionaryRef> messageBody = adoptWK(WKDictionaryCreate(rawKeys.data(), rawValues.data(), rawKeys.size()));

auto

> Tools/WebKitTestRunner/TestInvocation.cpp:1258
> +        WKRetainPtr<WKStringRef> subHostKey = adoptWK(WKStringCreateWithUTF8CString("SubHost"));

auto

> Tools/WebKitTestRunner/TestInvocation.cpp:1259
> +        WKRetainPtr<WKStringRef> topHostKey = adoptWK(WKStringCreateWithUTF8CString("TopHost"));

auto

> Tools/WebKitTestRunner/TestInvocation.cpp:1265
> +        WKRetainPtr<WKTypeRef> result = adoptWK(WKBooleanCreate(statisticInDatabaseOnce));

No need for this local variable, just return right away.

> LayoutTests/http/tests/resourceLoadStatistics/many-inserts-only-insert-once-expected.txt:1
> +fake test.

This is surprising.
Comment 4 Katherine_cheney 2019-10-07 14:00:45 PDT
Created attachment 380359 [details]
Patch
Comment 5 WebKit Commit Bot 2019-10-07 15:37:07 PDT
Comment on attachment 380359 [details]
Patch

Rejecting attachment 380359 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 380359, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as commit-queue@webkit.org...
Fetching: https://bugs.webkit.org/attachment.cgi?id=380359&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=202604&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Updating working directory
Processing patch 380359 from bug 202604.
Fetching: https://bugs.webkit.org/attachment.cgi?id=380359
Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	A	LayoutTests/http/tests/resourceLoadStatistics/many-inserts-only-insert-once-expected.txt
	A	LayoutTests/http/tests/resourceLoadStatistics/many-inserts-only-insert-once.html
	M	LayoutTests/ChangeLog

ERROR from SVN:
Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date
W: f4be6607737832e584d6ba23b1ccf02997d789b2 and refs/remotes/origin/master differ, using rebase:
:040000 040000 3ec1adaa6f75c76fb7becffddc2c5634a28af7bf d3143ee2d72080ff4d75c7d4f0b4a9efb3ab75ba M	LayoutTests
:040000 040000 c0ab4c71d29e60301eab11c5e9afb96ed762d678 c33f1ad251eb0fc2dcd5a42100fe485d0dc0fad0 M	Source
:040000 040000 84fe9d257a3dda72ec3a1886134a1ef6c49e7d59 ce9714c222cdcb45fd489a7b646c3591607e54e5 M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	A	LayoutTests/http/tests/resourceLoadStatistics/many-inserts-only-insert-once-expected.txt
	A	LayoutTests/http/tests/resourceLoadStatistics/many-inserts-only-insert-once.html
	M	LayoutTests/ChangeLog

ERROR from SVN:
Item is out of date: File '/trunk/LayoutTests/ChangeLog' is out of date
W: f4be6607737832e584d6ba23b1ccf02997d789b2 and refs/remotes/origin/master differ, using rebase:
:040000 040000 3ec1adaa6f75c76fb7becffddc2c5634a28af7bf d3143ee2d72080ff4d75c7d4f0b4a9efb3ab75ba M	LayoutTests
:040000 040000 c0ab4c71d29e60301eab11c5e9afb96ed762d678 c33f1ad251eb0fc2dcd5a42100fe485d0dc0fad0 M	Source
:040000 040000 84fe9d257a3dda72ec3a1886134a1ef6c49e7d59 ce9714c222cdcb45fd489a7b646c3591607e54e5 M	Tools
Current branch master is up to date.
ERROR: Not all changes have been committed into SVN, however the committed
ones (if any) seem to be successfully integrated into the working tree.
Please see the above messages for details.


Failed to run "['git', 'svn', 'dcommit', '--rmdir']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit
Updating OpenSource
From https://git.webkit.org/git/WebKit
   293423b2e3b..9cd68bb2a10  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 250795 = 293423b2e3bc99a3b6b08c9e39c59d8eb89f00e1
r250797 = 368609aca428fc8204b022c574a064ef2eb32b94
r250798 = 27eaa97abcb975632a99166f495de0aa6ad7aa83
r250799 = 9cd68bb2a10e57ff00cd6c464c483b0f2adf559b
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: https://webkit-queues.webkit.org/results/13104128
Comment 6 Katherine_cheney 2019-10-07 15:56:58 PDT
Created attachment 380370 [details]
Patch
Comment 7 Katherine_cheney 2019-10-07 15:59:10 PDT
Did a rebase and uploaded again.
Comment 8 WebKit Commit Bot 2019-10-07 16:17:36 PDT
Comment on attachment 380370 [details]
Patch

Clearing flags on attachment: 380370

Committed r250804: <https://trac.webkit.org/changeset/250804>
Comment 9 WebKit Commit Bot 2019-10-07 16:17:37 PDT
All reviewed patches have been landed.  Closing bug.