[webkit-reviews] review denied: [Bug 173998] IndexedDB: ensure its backward compatibility : [Attachment 315294] Patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has denied Jiewen Tan <jiewen_tan at apple.com>'s
request for review:
Bug 173998: IndexedDB: ensure its backward compatibility
https://bugs.webkit.org/show_bug.cgi?id=173998

Attachment 315294: Patch

https://bugs.webkit.org/attachment.cgi?id=315294&action=review




--- Comment #8 from Brady Eidson <beidson at apple.com> ---
Comment on attachment 315294
  --> https://bugs.webkit.org/attachment.cgi?id=315294
Patch

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

In general, please re-write using the Cocoa API, then I'll look closer at the
HTML.

> Source/WebCore/ChangeLog:3
> +	   IndexedDB: ensure its backward compatibility

This is not really descriptive.

I'd say something like "Ensure backward compatibility in the serialization
format of structured clones."

> Source/WebCore/bindings/js/SerializedScriptValue.cpp:120
> +// Who edits this list, please teach IndexedDB.BackwardCompatibility API
test to learn the new stuff.

"When making changes to this list please cover your new type(s) in the API test
"IndexedDB.BackwardCompatibility"

> Tools/ChangeLog:8
> +	   A test is composed to ensure IndexedDB's backward compatibility. The
way it works is to

Again, this is not really about "IndexedDB backward compatibility" as it is
about "backward compatibility in the serialization format of structured clones"

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/IndexedDBBackwardCompatibility.mm:73
> +    // Somehow ~/Library/WebKit/TestWebKitAPI/WebsiteData/IndexedDB/ doesn't
work on my machine. @Brady any idea?
> +    NSURL *targetURL = [NSURL
fileURLWithPath:[@"~/Library/WebKit/Databases/___IndexedDB/file__0/backward_com
patibility/" stringByExpandingTildeInPath]];
> +    [[NSFileManager defaultManager] createDirectoryAtURL:targetURL
withIntermediateDirectories:YES attributes:nil error:nil];

You need to use the Cocoa API and specify an IndexedDB directory location in
the WKWebsiteDataStoreConfiguration.

(See comment below on other reasons to not use the C API)

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/IndexedDBBackwardCompatibility.mm:86
> +    WKPageNavigationClientV0 navigationClient;
> +    memset(&navigationClient, 0, sizeof(navigationClient));
> +    navigationClient.base.version = 0;
> +    navigationClient.decidePolicyForNavigationAction =
decidePolicyForNavigationAction;
> +    navigationClient.decidePolicyForNavigationResponse =
decidePolicyForNavigationResponse;
> +    navigationClient.copyWebCryptoMasterKey = copyWebCryptoMasterKey;

The Cocoa API is the future, and the C-API will eventually be removed.

Writing this test using the C-API means it will just have to be re-written
later.

Please re-write now using the Cocoa API