Bug 141634 - Web Inspector: ES6: Improved Console Support for Promise Objects
Summary: Web Inspector: ES6: Improved Console Support for Promise Objects
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on: 141746
Blocks:
  Show dependency treegraph
 
Reported: 2015-02-16 00:01 PST by Joseph Pecoraro
Modified: 2015-02-17 23:59 PST (History)
11 users (show)

See Also:


Attachments
[PATCH] Work In Progress (16.71 KB, patch)
2015-02-16 00:06 PST, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] Proposed Fix (19.28 KB, patch)
2015-02-16 13:51 PST, Joseph Pecoraro
timothy: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (628.67 KB, application/zip)
2015-02-16 14:19 PST, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (676.04 KB, application/zip)
2015-02-16 14:27 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-02-16 00:01:49 PST
* SUMMARY
Improved Console Support for Promise Objects.

Promises have internal state that would be useful to show in Inspector: (Table 59 — Internal Slots of Promise Instances)

[[PromiseState]]
  - A string value that governs how a promise will react to incoming calls to its then method.
  - The possible values are: undefined, "pending", "fulfilled", and "rejected".

[[PromiseResult]]
  - The value with which the promise has been fulfilled or rejected, if any.
  - Only meaningful if [[PromiseState]] is not "pending".

[[PromiseFulfillReactions]]
  - A List of PromiseReaction records to be processed when/if the promise transitions from the "pending" state to the "fulfilled" state.

[[PromiseRejectReactions]]
  - A List of PromiseReaction records to be processed when/if the promise transitions from the "pending" state to the "rejected" state.
Comment 1 Radar WebKit Bug Importer 2015-02-16 00:02:10 PST
<rdar://problem/19843138>
Comment 2 Joseph Pecoraro 2015-02-16 00:06:47 PST
Created attachment 246637 [details]
[PATCH] Work In Progress

Work in progress patch:
- First time we make sure of InjectedScriptHost.getInternalProperties
- Add a way to distinguish Internal property previews from regular properties
- Trivially supports [[PromiseState]] and [[PromiseResult]].

Whats next:
- show the chain of reactions next (the .thens, and .catches)
- discuss UI for Internal Properties in ObjectPreviews / ObjectTrees
Comment 3 Joseph Pecoraro 2015-02-16 13:47:01 PST
> [[PromiseFulfillReactions]]
>   - A List of PromiseReaction records to be processed when/if the promise
> transitions from the "pending" state to the "fulfilled" state.
> 
> [[PromiseRejectReactions]]
>   - A List of PromiseReaction records to be processed when/if the promise
> transitions from the "pending" state to the "rejected" state.

I will handle these states in:
<https://webkit.org/b/141664> Web Inspector: ES6: Improved Support for Promises - Promise Reactions

For now, lets just do state / value.
Comment 4 Joseph Pecoraro 2015-02-16 13:51:02 PST
Created attachment 246675 [details]
[PATCH] Proposed Fix
Comment 5 Build Bot 2015-02-16 14:19:00 PST
Comment on attachment 246675 [details]
[PATCH] Proposed Fix

Attachment 246675 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5867389241524224

New failing tests:
inspector-protocol/runtime/getProperties.html
Comment 6 Build Bot 2015-02-16 14:19:03 PST
Created attachment 246682 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 7 Build Bot 2015-02-16 14:27:54 PST
Comment on attachment 246675 [details]
[PATCH] Proposed Fix

Attachment 246675 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4929187995451392

New failing tests:
inspector-protocol/runtime/getProperties.html
Comment 8 Build Bot 2015-02-16 14:27:57 PST
Created attachment 246683 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 9 Joseph Pecoraro 2015-02-16 14:28:45 PST
Comment on attachment 246675 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/inspector/InjectedScript.cpp:139
> -    auto array = BindingTraits<Array<Inspector::Protocol::Runtime::InternalPropertyDescriptor>>::runtimeCast(WTF::move(result));
> -    *properties = array->length() > 0 ? array : nullptr;
> +    *properties = BindingTraits<Array<Inspector::Protocol::Runtime::InternalPropertyDescriptor>>::runtimeCast(WTF::move(result));

I should revert this part for:
spector-protocol/runtime/getProperties.html

I wanted to always include the array, but not including it if the array is empty will simplify the response.
Comment 10 Timothy Hatcher 2015-02-16 14:41:03 PST
Comment on attachment 246675 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:200
> +    result->putDirect(exec->vm(), Identifier(exec, "name"), jsString(exec, name));

jsNontrivialString and ASCIILiteral for name?
Comment 11 Joseph Pecoraro 2015-02-16 14:47:33 PST
Comment on attachment 246675 [details]
[PATCH] Proposed Fix

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

>> Source/JavaScriptCore/inspector/JSInjectedScriptHost.cpp:200
>> +    result->putDirect(exec->vm(), Identifier(exec, "name"), jsString(exec, name));
> 
> jsNontrivialString and ASCIILiteral for name?

jsNontrivialString - I debated it, and said no in case we ever have a single character string. (it asserts length > 1, not >= 1).
ASCIILiteral - I thought there was a jsString for char* but there isn't. I'll change the param to const String& and use ASCIILiteral above.
Comment 12 Joseph Pecoraro 2015-02-17 12:18:13 PST
http://trac.webkit.org/changeset/180235
Comment 13 Csaba Osztrogonác 2015-02-17 23:59:24 PST
(In reply to comment #12)
> http://trac.webkit.org/changeset/180235

It broke the !ENABLE(PROMISES) build, see bug141746 for details.