Bug 167473 – Uncaught Exception: Can't make a ContentView for an unknown representedObject of type: IndexedDatabase

• Component: Web Inspector
• Status: RESOLVED FIXED

[John Wilander]

-------
Inspected URL:        Local page
Loading completed:    true
Frontend User Agent:  Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/604.1.5+ (KHTML, like Gecko)

Uncaught Exceptions:
 - Can't make a ContentView for an unknown representedObject of type: IndexedDatabase (at ContentView.js:152:24)
    createFromRepresentedObject @ ContentView.js:152:24
    contentViewForRepresentedObject @ ContentView.js:173:82
    showContentViewForRepresentedObject @ ContentBrowser.js:134:63
    showDefaultContentViewForTreeElement @ NavigationSidebarPanel.js:223:82
    _checkElementsForPendingViewStateCookie @ NavigationSidebarPanel.js:781:79
    _treeElementAddedOrChanged @ NavigationSidebarPanel.js:638:57
    dispatch @ Object.js:170:30
    dispatchEventToListeners @ Object.js:177:17
    appendChild @ TreeOutline.js:177:54
    _addIndexedDatabase @ StorageSidebarPanel.js:261:47
    _indexedDatabaseWasAdded @ StorageSidebarPanel.js:246:33
    dispatch @ Object.js:170:30
    dispatchEventToListeners @ Object.js:177:17
    processDatabase @ StorageManager.js:275:42
    processDatabase @ [native code]
    _dispatchResponseToCallback @ Connection.js:146:27
    _dispatchResponse @ Connection.js:116:45
    dispatch @ Connection.js:69:35
    dispatch @ InspectorBackend.js:151:49
    dispatchNextQueuedMessageFromBackend @ MessageDispatcher.js:42:34

Additional Details:
    cause --> An uncaught exception was thrown while dispatching response callback for command IndexedDB.requestDatabase.
-------

* STEPS TO REPRODUCE
1. The data records for the domain had been cleared, including IndexedDB.
2. The IndexedDB instance still showed in Web Inspector | Storage. I clicked it and got the crash.

[Blaze Burg]

Thanks John, we are probably just missing a case for this.

[Radar WebKit Bug Importer]

<rdar://problem/30249715>

[Nikita Vasilyev]

Steps:
1. Open http://www.girliemac.com/stickies/
2. Click on "New note...", type "foo", press Enter
3. In Web Inspector Storage tab, expand Indexed Databases
4. Click "stickieDB"
5. Close Web Inspector
6. Open Web Inspector

[Nikita Vasilyev]

Created attachment 308617 [details]
[Animated GIF] Missing content view

There's no content view for:
1. Indexed Databases folder.
2. Indexed Database root item.

Currently, we only have a content view for an IndexedDB table.

[Nikita Vasilyev]

Created attachment 308621 [details]
[Image] Chrome DevTools IndexedDB content view

Chrome DevTools show a DB name, security origin, and a version. There's also a button to delete a DB. We should do something like this, too.

[Joseph Pecoraro]

> Chrome DevTools show a DB name, security origin, and a version. There's also
> a button to delete a DB. We should do something like this, too.

We should be showing must of this information in a context sensitive sidebar.

[Nikita Vasilyev]

(In reply to Joseph Pecoraro from comment #6)
> > Chrome DevTools show a DB name, security origin, and a version. There's also
> > a button to delete a DB. We should do something like this, too.
> 
> We should be showing must of this information in a context sensitive sidebar.

Yes, we do show all of it in the sidebar.

I think it's strange to switch from, say, Cookies to an indexed database and not update the content view. It could be misleading.

[Nikita Vasilyev]

Created attachment 308642 [details]
Patch

[Nikita Vasilyev]

Created attachment 308643 [details]
[Image] With patch applied

[Joseph Pecoraro]

What right sidebars are available with this patch? Is the IndexedDB sidebar available?

[Nikita Vasilyev]

Created attachment 308656 [details]
[Image] Details sidebar

(In reply to Joseph Pecoraro from comment #10)
> What right sidebars are available with this patch? Is the IndexedDB sidebar
> available?

Maybe we shouldn't show a sidebar here at all.

[Nikita Vasilyev]

(In reply to Nikita Vasilyev from comment #11)
> Created attachment 308656 [details]
> [Image] Details sidebar
> 
> (In reply to Joseph Pecoraro from comment #10)
> > What right sidebars are available with this patch? Is the IndexedDB sidebar
> > available?
> 
> Maybe we shouldn't show a sidebar here at all.

I'm not sure how to do this. When I remove WebInspector.IndexedDatabase from WebInspector.StorageTabContentView#canShowRepresentedObject clicking on the indexed database item in the sidebar ("stickieDB" on the screenshot) doesn't change the content view any longer :/

[Nikita Vasilyev]

(In reply to Nikita Vasilyev from comment #12)
> (In reply to Nikita Vasilyev from comment #11)
> > Created attachment 308656 [details]
> > [Image] Details sidebar
> > 
> > (In reply to Joseph Pecoraro from comment #10)
> > > What right sidebars are available with this patch? Is the IndexedDB sidebar
> > > available?
> > 
> > Maybe we shouldn't show a sidebar here at all.
> 
> I'm not sure how to do this. When I remove WebInspector.IndexedDatabase from
> WebInspector.StorageTabContentView#canShowRepresentedObject clicking on the
> indexed database item in the sidebar ("stickieDB" on the screenshot) doesn't
> change the content view any longer :/

Sorry, ignore this comment, it doesn't make a lot of sense :)

[Matt Baker]

(In reply to Nikita Vasilyev from comment #13)
> (In reply to Nikita Vasilyev from comment #12)
> > (In reply to Nikita Vasilyev from comment #11)
> > > Created attachment 308656 [details]
> > > [Image] Details sidebar
> > > 
> > > (In reply to Joseph Pecoraro from comment #10)
> > > > What right sidebars are available with this patch? Is the IndexedDB sidebar
> > > > available?
> > > 
> > > Maybe we shouldn't show a sidebar here at all.
> > 
> > I'm not sure how to do this. When I remove WebInspector.IndexedDatabase from
> > WebInspector.StorageTabContentView#canShowRepresentedObject clicking on the
> > indexed database item in the sidebar ("stickieDB" on the screenshot) doesn't
> > change the content view any longer :/
> 
> Sorry, ignore this comment, it doesn't make a lot of sense :)

The confusion arises because we continue to show the view for current represented object when selecting a tree item without a valid represented object. I think your patch helps address this.

Will review more thoroughly.

[Nikita Vasilyev]

Created attachment 308787 [details]
Patch

Same patch, except the details sidebar for IndexedDB top level item is always hidden since it has the same content as the content view.

[Matt Baker]

Created attachment 308796 [details]
[Image] Other UI ideas

I like the IndexedDatabaseContentView added by this patch. In the future we may want to have it serve as a summary/dashboard for the whole database (see attached mockup).

There is some redundant information already contained in the breadcrumbs/sidebar, but might still be useful to think about for a future patch.

[Matt Baker]

Comment on attachment 308787 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:9
> +        When an indexed database is selected in the Network navigation sidebar, show its host, security origin, and version.

I think you mean Storage navigation sidebar.

> Source/WebInspectorUI/ChangeLog:11
> +        Previously, selecting an indexed database didn't change the content view. It could led to a misleading state

typo: lead

> Source/WebInspectorUI/ChangeLog:12
> +        when an indexed database is selected in the sidebar, but the content view showed previosly selected item such as

typo: previously

> Source/WebInspectorUI/UserInterface/Main.html:513
> +    <script src="Views/CloseButtonView.js"></script>

Looks like it was included by mistake.

[Matt Baker]

You could do this with a DetailsSection directly in the ContentView. It would be easy then to add groups for object stores, etc, in the future.

To get the UI as you have it now, you'll want to hide the DetailsSection's header in CSS, and add some styles to ContentView.css to make it prettier:

.content-view .details-section {
    font-size: 11px;
    background-color: initial;
}

.content-view .details-section > .header {
    font-size: 13px;
    font-weight: 600;
    background-color: initial;
}

.content-view .details-section > .content > .group > .row.simple > .label {
    width: 120px;
    font-weight: 600;
}

.content-view .details-section > .content > .group > .row.simple > .value {
    color: var(--text-color-gray-medium);
}

.content-view .details-section:last-child {
    border-bottom: none;
}

[Matt Baker]

And I think you'll need to adjust the spacing around the section itself.

[Nikita Vasilyev]

(In reply to Matt Baker from comment #16)
> Created attachment 308796 [details]
> [Image] Other UI ideas
> 
> I like the IndexedDatabaseContentView added by this patch. In the future we
> may want to have it serve as a summary/dashboard for the whole database (see
> attached mockup).
> 
> There is some redundant information already contained in the
> breadcrumbs/sidebar, but might still be useful to think about for a future
> patch.

I can't find larger versions of Database.png and DatabaseTable.png to use in the content view.

Also, these icons are pre iOS 7 style, e.g. they aren't flat. We should update them at some point.

[Matt Baker]

(In reply to Nikita Vasilyev from comment #20)
> (In reply to Matt Baker from comment #16)
> > Created attachment 308796 [details]
> > [Image] Other UI ideas
> > 
> > I like the IndexedDatabaseContentView added by this patch. In the future we
> > may want to have it serve as a summary/dashboard for the whole database (see
> > attached mockup).
> > 
> > There is some redundant information already contained in the
> > breadcrumbs/sidebar, but might still be useful to think about for a future
> > patch.
> 
> I can't find larger versions of Database.png and DatabaseTable.png to use in
> the content view.

I think your original UI, using a DetailsSection rather than a custom table, is fine for this patch.

> Also, these icons are pre iOS 7 style, e.g. they aren't flat. We should
> update them at some point.

Agree. I'd like something vector and monochrome (or at least flat colors).

[Nikita Vasilyev]

Created attachment 308868 [details]
Patch

[Nikita Vasilyev]

Created attachment 308869 [details]
[Image] With patch applied

[Matt Baker]

Comment on attachment 308868 [details]
Patch

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

Looks good! r=me, with comments.

> Source/WebInspectorUI/UserInterface/Views/ContentView.css:45
> +}

This rule should go in IndexedDatabaseContentView.css. Headers will be nice to have when we have multiple sections in a ContentView.

> Source/WebInspectorUI/UserInterface/Views/ContentView.css:52
> +    width: auto;

Much nicer than having a fixed width.

> Source/WebInspectorUI/UserInterface/Views/IndexedDatabaseContentView.js:47
> +    {

Can the represented object change during the lifetime of the view? If not this should move to constructor.

[Matt Baker]

Comment on attachment 308868 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Views/IndexedDatabaseContentView.js:47
>> +    {
> 
> Can the represented object change during the lifetime of the view? If not this should move to constructor.

Layout is called repeatedly as the view is resized, so this will need to move before landing.

[Nikita Vasilyev]

Created attachment 308954 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 308954 [details]
Patch

Clearing flags on attachment: 308954

Committed r216147: <http://trac.webkit.org/changeset/216147>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.