Bug 112740 – Add a feature observer for RenderDeprecatedFlexibleBox

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Christian Biesinger]

Add a feature observer for RenderDeprecatedFlexibleBox

[Christian Biesinger]

Created attachment 193914 [details]
Patch

[Adam Barth]

LGTM, but Ojan should give you the official review.

[Tony Chang]

Comment on attachment 193914 [details]
Patch

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

> Source/WebCore/page/FeatureObserver.h:104
> +        DeprecatedFlexbox,

Nit: I would probably name this DeprecatedFlexboxWebContent or something.

[Christian Biesinger]

Created attachment 193944 [details]
Patch

[Eric Seidel (no email)]

Ojan was telling me today that FeatureObserver depends on normal-page shutdown, which is not how chromium renderers commonly shut down?  Suggesting we're not getting very good data from these metrics?

[WebKit Review Bot]

Comment on attachment 193944 [details]
Patch

Rejecting attachment 193944 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 193944, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
ebkit-commit-queue

Parsed 3 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/page/FeatureObserver.h
Hunk #1 FAILED at 101.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/page/FeatureObserver.h.rej
patching file Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://webkit-commit-queue.appspot.com/results/17191542

[Ojan Vafai]

(In reply to comment #5)
> Ojan was telling me today that FeatureObserver depends on normal-page shutdown, which is not how chromium renderers commonly shut down?  Suggesting we're not getting very good data from these metrics?

Chrome only does normal shutdown of the page if is being navigated to a same-domain page, it has beforeunload/unload listeners registered or there are multiple pages in the same render process (in all other cases, we can safely just kill the render process). It's kind of a bummer, but I'm not sure how we would be able to get better metrics. Hopefully this doesn't skew our numbers too much.

[Christian Biesinger]

Created attachment 194092 [details]
Patch

Rebased

[WebKit Review Bot]

Comment on attachment 194092 [details]
Patch

Clearing flags on attachment: 194092

Committed r146375: <http://trac.webkit.org/changeset/146375>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.