Hook up IndexedDB inspection.
<rdar://problem/16136778>
Created attachment 224923 [details] Patch
Comment on attachment 224923 [details] Patch Attachment 224923 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5784519332331520 New failing tests: inspector-protocol/storage/indexed-database.html
Created attachment 224927 [details] Archive of layout-test-results from webkit-ews-01 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-01 Port: mac-mountainlion Platform: Mac OS X 10.8.5
Comment on attachment 224923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224923&action=review r=me > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStore.js:27 > +WebInspector.IndexedDatabaseObjectStore = function(name, keyPath, autoIncrement, indexes) > +{ Nit: Missing super constructor call. WebInspector.Object.call(this) > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStore.js:61 > + get autoIncrement() > + { > + return this._autoIncrement; > + }, Should we show something in the UI if this is an auto incrementing Object Store? It seems unused right now. > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreContentView.css:62 > + /* FIXME: This should show a alternating stripe, but I couldn't make it work. */ Typo: "a alternating" => "an alternating" > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreContentView.js:113 > + for (var entry of this._entries) { > + entry.primaryKey.release(); > + entry.key.release(); > + entry.value.release(); > + } Arg, this looks horrible to me. If there are 100 rows, this means we would need to send 300 messages to the backend to clear all these RemoteObjects? We should put these in a RemoteObject group and just send one message to clear the group. More on that below. I don't think this blocks landing this patch, but it would be a good follow-up. > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreIndex.js:63 > + get unique() > + { > + return this._unique; > + }, > + > + get multiEntry() > + { > + return this._multiEntry; > + }, Should we show these in the UI somehow? > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreIndexTreeElement.js:2 > + * Copyright (C) 2013 Apple Inc. All rights reserved. Nit: 2014 > Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreTreeElement.js:2 > + * Copyright (C) 2013 Apple Inc. All rights reserved. Nit: 2014 > Source/WebInspectorUI/UserInterface/StorageManager.js:36 > + if (window.IndexedDBAgent) > + IndexedDBAgent.enable(); I think this will be mysterious on iOS 6 and iOS 7. Though this is in the protocol for iOS 6 and iOS 7, the feature wasn't enabled. I think we should remove the "IndexedDB" domain from the iOS 6 and iOS 7 versions files so that feature detection works as expected there: Source/WebInspectorUI/Versions/Inspector-iOS-6.0.json Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json > Source/WebInspectorUI/UserInterface/StorageManager.js:168 > + IndexedDBAgent.requestData.invoke(requestArguments, processData.bind(this)); Nit: The .bind(this) is not needed. "this" is not use din the "processData". We could be passing a "group id" string from here to the backend so we have a group to clear the resulting RemoteObjects all at once. This eventually gets down to the following code in class OpenCursorCallback in InspectorIndexedDBAgent.cpp: RefPtr<DataEntry> dataEntry = DataEntry::create() .setKey(m_injectedScript.wrapObject(idbCursor->key(), String())) .setPrimaryKey(m_injectedScript.wrapObject(idbCursor->primaryKey(), String())) .setValue(m_injectedScript.wrapObject(idbCursor->value(), String())); If instead of String() if we had provided a group name, we could use that! > Source/WebInspectorUI/UserInterface/StorageManager.js:249 > + var objectStores = databasePayload.objectStores.map(processObjectStore.bind(this)); A bunch of the .bind(this)s are not needed here. But I'll leave removing them up to you. > LayoutTests/inspector-protocol/storage/indexed-database.html:1 > +<!DOCTYPE html> I'd prefer the file path specify the domain. We've been going with that kind of hierarchy where possible, but maybe it is time to abandon it: LayoutTests/inspector-protocol/<domain>/<command>.html LayoutTests/inspector-protocol/indexeddb/something.html
Comment on attachment 224923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=224923&action=review >> Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStore.js:61 >> + }, > > Should we show something in the UI if this is an auto incrementing Object Store? It seems unused right now. We could in the future. Maybe a details sidebar? >> Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreContentView.js:113 >> + } > > Arg, this looks horrible to me. > > If there are 100 rows, this means we would need to send 300 messages to the backend to clear all these RemoteObjects? We should put these in a RemoteObject group and just send one message to clear the group. More on that below. > > I don't think this blocks landing this patch, but it would be a good follow-up. Yeah, I was surprised by this too. I filed bug 129203. >> Source/WebInspectorUI/UserInterface/IndexedDatabaseObjectStoreIndex.js:63 >> + }, > > Should we show these in the UI somehow? Maybe a details sidebar too? >> Source/WebInspectorUI/UserInterface/StorageManager.js:36 >> + IndexedDBAgent.enable(); > > I think this will be mysterious on iOS 6 and iOS 7. Though this is in the protocol for iOS 6 and iOS 7, the feature wasn't enabled. I think we should remove the "IndexedDB" domain from the iOS 6 and iOS 7 versions files so that feature detection works as expected there: > > Source/WebInspectorUI/Versions/Inspector-iOS-6.0.json > Source/WebInspectorUI/Versions/Inspector-iOS-7.0.json Good idea. Fixed. >> LayoutTests/inspector-protocol/storage/indexed-database.html:1 >> +<!DOCTYPE html> > > I'd prefer the file path specify the domain. We've been going with that kind of hierarchy where possible, but maybe it is time to abandon it: > > LayoutTests/inspector-protocol/<domain>/<command>.html > LayoutTests/inspector-protocol/indexeddb/something.html Done.
https://trac.webkit.org/changeset/164541