Bug 47817 - FrameLoader::IsProcessingUserGesture is true for JavaScript initiated downloads after click navigation to webpage
Summary: FrameLoader::IsProcessingUserGesture is true for JavaScript initiated downloa...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-18 06:31 PDT by Pierre-Antoine LaFayette
Modified: 2010-12-02 12:25 PST (History)
7 users (show)

See Also:


Attachments
links.html, links to wait.html (215 bytes, text/html)
2010-10-18 06:31 PDT, Pierre-Antoine LaFayette
no flags Details
wait.html, when loaded it will download setup.exe after 5 secs. (216 bytes, text/html)
2010-10-18 06:32 PDT, Pierre-Antoine LaFayette
no flags Details
Patch (1.85 KB, patch)
2010-10-20 07:07 PDT, Pierre-Antoine LaFayette
abarth: review-
Details | Formatted Diff | Diff
patch v1 (5.82 KB, patch)
2010-10-20 17:46 PDT, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff
patch v1 (with layout tests) (6.97 KB, patch)
2010-10-20 22:45 PDT, Johnny(Jianning) Ding
abarth: review-
Details | Formatted Diff | Diff
patch v2 (6.56 KB, patch)
2010-10-21 00:17 PDT, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff
re-baselined landing patch, (7.39 KB, patch)
2010-11-09 07:06 PST, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff
re-baselined landing patch v2 (7.38 KB, patch)
2010-11-18 01:31 PST, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff
patch to resolve the latest conflict (7.43 KB, patch)
2010-11-30 16:51 PST, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff
Patch for landing (7.43 KB, patch)
2010-11-30 16:58 PST, Johnny(Jianning) Ding
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Antoine LaFayette 2010-10-18 06:31:05 PDT
Created attachment 71028 [details]
links.html, links to wait.html

If I click on a link that navigates to a page which uses setTimeout to initiate a download (e.g. setTimeout("window.location = 'file.exe'", 5000)), FrameLoader::IsProcessingUserGesture is still true when loading of file.exe begins.

It seems that the UserGestureState is set to PossiblyProcessingUserGesture and the !activeFrame conditional gets hit.

bool ScriptController::processingUserGesture()
{
    Frame* activeFrame = V8Proxy::retrieveFrameForEnteredContext();
    // No script is running, so it is user-initiated unless the gesture stack
    // explicitly says it is not.
    if (!activeFrame)
        return UserGestureIndicator::getUserGestureState() != DefinitelyNotProcessingUserGesture;
Comment 1 Pierre-Antoine LaFayette 2010-10-18 06:32:30 PDT
Created attachment 71029 [details]
wait.html, when loaded it will download setup.exe after 5 secs.
Comment 2 Pierre-Antoine LaFayette 2010-10-18 06:57:31 PDT
This was observed in Chromium. RenderView::didStartProvisionalLoad sets its |navigation_gesture_| based on the results of FrameLoader::IsProcessingUserGesture.
Comment 3 Adam Barth 2010-10-18 09:47:56 PDT
RenderView::didStartProvisionalLoad doesn't exist in svn.webkit.org.  Is there an issue that's possible to observe using code from svn.webkit.org?
Comment 4 Johnny(Jianning) Ding 2010-10-18 11:43:48 PDT
(In reply to comment #3)
> RenderView::didStartProvisionalLoad doesn't exist in svn.webkit.org.  Is there an issue that's possible to observe using code from svn.webkit.org?

Hi Adam,  Pierre-Antoine was right, the WebFrameImpl::isProcessingUserGesture() did return true (met PossiblyProcessingUserGesture and !activeFrame conditions) when the page was redirected (triggered by setTimeout) to file.exe.

It was because we didn't explicit set the GestureIndicator to DefinitelyProcessingUserGesture/DefinitelyNotProcessingUserGesture acording to the m_wasUserGesture in ScheduledURLNavigation::fire. (We only passed the m_wasUserGesture to FrameLoader::changeLocation).

I will file a patch to fix this bug.
Comment 5 Pierre-Antoine LaFayette 2010-10-18 11:50:42 PDT
I believe this issue is only reproducible because we are checking the state of the user gesture in the FrameLoader during the load. But as Johnny pointed out, the issue seems to still be within the WebKit code.
Comment 6 Adam Barth 2010-10-18 12:26:55 PDT
> I will file a patch to fix this bug.

Great.  We'll need to find a way to write a test for the fix...
Comment 7 Johnny(Jianning) Ding 2010-10-18 12:31:33 PDT
(In reply to comment #6)
> > I will file a patch to fix this bug.
> 
> Great.  We'll need to find a way to write a test for the fix...
That's pain because we can not write script o test the gesture status (since using script breaks the !activeFrame condition). 
For chrome side, a ui test may be a way, so far no idea about webkit test.
Comment 8 Adam Barth 2010-10-18 12:34:02 PDT
> That's pain because we can not write script o test the gesture status (since using script breaks the !activeFrame condition). 
> For chrome side, a ui test may be a way, so far no idea about webkit test.

Yeah, I think a UI test is ok in this case...  I'm not sure how we could test it in DumpRenderTree...
Comment 9 Alexey Proskuryakov 2010-10-18 12:39:43 PDT
Is the problem that we cannot simulate a click without JS on the stack?
Comment 10 Adam Barth 2010-10-18 13:10:16 PDT
(In reply to comment #9)
> Is the problem that we cannot simulate a click without JS on the stack?

My understanding is the problem is that we can't read the state in question without JavaScript on the stack.  It's visible in the browser not as a web compat issue but as wrong behavior by the download manager.
Comment 11 Johnny(Jianning) Ding 2010-10-18 14:00:10 PDT
(In reply to comment #8) 
> Yeah, I think a UI test is ok in this case...  I'm not sure how we could test it in DumpRenderTree...

I figured out a way to test it in DumpRenderTree on all ports.

We call LayoutController::dumpFrameLoadCallbacks() to dump the frame info when calling didStartProvisionalLoad logic. Please refer to (only lists mac/chromium ports)
1. https://svn.webkit.org/browser/trunk/WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp#L1290
2. https://svn.webkit.org/browser/trunk/WebKitTools/DumpRenderTree/mac/FrameLoadDelegate.mm#L143

In the dump implementation, we output the gesture status of main frame.  Please refer to (only lists mac/chromium ports)
1. https://svn.webkit.org/browser/trunk/WebKitTools/DumpRenderTree/chromium/WebViewHost.cpp#L841
2. https://svn.webkit.org/browser/trunk/WebKitTools/DumpRenderTree/mac/FrameLoadDelegate.mm#L76

Since the way needs to change a few DumpRenderTree files, I'd like file another bug to address the test issue and only provide the code patch in here.
Adam, does it make sense to you?
Comment 12 Adam Barth 2010-10-18 14:10:46 PDT
That's a good idea.  We'll want to land the test infrastructure before the code change, of course.  :)
Comment 13 Johnny(Jianning) Ding 2010-10-18 14:23:42 PDT
(In reply to comment #12)
> That's a good idea.  We'll want to land the test infrastructure before the code change, of course.  :)

Thanks. Bug 47849 is filed to track the test infrastructure.
Comment 14 Johnny(Jianning) Ding 2010-10-19 06:43:06 PDT
Just found Adam's r69924 (https://svn.webkit.org/changeset/69924) already fixed this issue, now all fire implementations set the right UserGestureIndicator based on m_wasUserGesture.

Adam, please close this bug. I will update the test infrastructure progress in bug 47849.
Comment 15 Pierre-Antoine LaFayette 2010-10-19 06:51:11 PDT
Nice.
Comment 16 Pierre-Antoine LaFayette 2010-10-20 07:07:48 PDT
Created attachment 71290 [details]
Patch
Comment 17 Pierre-Antoine LaFayette 2010-10-20 07:13:33 PDT
Hi I'm reopening this bug because Adam's patch does not fix the case where a user initiated navigation does a meta refresh and triggers a download; which, as discussed, should not be considered a user initiated navigation. 

To me it would seem that we should have IsProcessingUserGesture only return true if we know for certain that the navigation was user initiated. 

Adam, can you please review my patch to assure that I'm not overlooking anything by this change.

Thanks.
Comment 18 Johnny(Jianning) Ding 2010-10-20 08:33:50 PDT
(In reply to comment #17)
> Hi I'm reopening this bug because Adam's patch does not fix the case where a user initiated navigation does a meta refresh and triggers a download; which, as discussed, should not be considered a user initiated navigation. 
> 
> To me it would seem that we should have IsProcessingUserGesture only return true if we know for certain that the navigation was user initiated. 
> 
> Adam, can you please review my patch to assure that I'm not overlooking anything by this change.
> 
> Thanks.

@Pierre-Antoine LaFayette, your patch is not the right way.

The default status of GestureIndicator is PossiblyProcessingUserGesture, some user-initiated actions don't explicitly pass the key/mouse events to WebKit (like typing URL in address bar and press enter), so in this case, we need to use the combination of GestureIndicator!=PossiblyProcessingUserGesture and no-js-on-stack to ensure this kind of user-gesture actions.


Yes, in current WebKit trunk, the meta refresh redirection caused  that FrameLoader::IsProcessingUserGesture returned true when calling it in WebViewClient::didStartProvisionalLoad, but the root cause is not what your patch pointed.

The bug was introduced by Adam's r47786(https://bugs.webkit.org/show_bug.cgi?id=47786).
In old WebKit code, the scheduleRefresh explicitly set false as user gesture status (meta refresh never can be considered as user initiated). However Adam might forget to keep this logic in NavigationScheduler::scheduleRedirect  (https://bugs.webkit.org/attachment.cgi?id=70988&action=diff#WebCore/loader/NavigationScheduler.cpp_sec6)

I will handle the patch once I finish https://bugs.webkit.org/show_bug.cgi?id=47849. Should be done today
Comment 19 Johnny(Jianning) Ding 2010-10-20 08:37:44 PDT
> The bug was introduced by Adam's r47786(https://bugs.webkit.org/show_bug.cgi?id=47786).
> In old WebKit code, the scheduleRefresh explicitly set false as user gesture status (meta refresh never can be considered as user initiated). However Adam might forget to keep this logic in NavigationScheduler::scheduleRedirect  (https://bugs.webkit.org/attachment.cgi?id=70988&action=diff#WebCore/loader/NavigationScheduler.cpp_sec6)
> 
r47786 -> bug 47786, r69938(http://trac.webkit.org/changeset/69938)
Comment 20 Adam Barth 2010-10-20 12:20:38 PDT
Comment on attachment 71290 [details]
Patch

What happened to the testing infrastructure?
Comment 21 Johnny(Jianning) Ding 2010-10-20 13:11:47 PDT
(In reply to comment #20)
> (From update of attachment 71290 [details])
> What happened to the testing infrastructure?

Sent a patch, with using it we can track the gesture status in the frame load callbacks.
For example, the following test case can show that the user gesture is false in didStartProvisionalLoad callback. after fixing the meta-refresh bug.

<head>
<meta http-equiv="refresh" content="2">
</head>
<body>
<div id="loadCount"></div>
<div id="console"></div>
<script>      

    if (1 || window.layoutTestController) {
        if (!sessionStorage.loadCount)
            sessionStorage.loadCount = 1;
        else
            sessionStorage.loadCount = parseInt(sessionStorage.loadCount, 10) + 1;
        document.getElementById("loadCount").innerText = "load count : " + sessionStorage.loadCount;
        if (2 == sessionStorage.loadCount) {
            layoutTestController.notifyDone();
        } else {
            layoutTestController.dumpAsText();
            layoutTestController.dumpUserGestureInFrameLoadCallbacks();
            layoutTestController.waitUntilDone();
        }
    }
</script>

The result is
Frame with user gesture "true" - in didFinishDocumentLoadForFrame
Frame with user gesture "true" - in didHandleOnloadEventsForFrame
Frame with user gesture "true" - in willPerformClientRedirect
Frame with user gesture "true" - in didFinishLoadForFrame
Frame with user gesture "false" - in didStartProvisionalLoadForFrame
Frame with user gesture "true" - in didCancelClientRedirectForFrame
Frame with user gesture "true" - in didCommitLoadForFrame
Frame with user gesture "true" - in didFinishDocumentLoadForFrame
Frame with user gesture "true" - in didHandleOnloadEventsForFrame
Frame with user gesture "true" - in willPerformClientRedirect
Frame with user gesture "true" - in didFinishLoadForFrame
load count : 2
Comment 22 Johnny(Jianning) Ding 2010-10-20 17:46:39 PDT
Created attachment 71374 [details]
patch v1

This patch depends on patch in https://bugs.webkit.org/show_bug.cgi?id=47849
Comment 23 Johnny(Jianning) Ding 2010-10-20 17:48:45 PDT
(In reply to comment #22)
> This patch depends on patch in https://bugs.webkit.org/show_bug.cgi?id=47849

Miss test exceptions in skip list for GTK/win ports. Will add it later
Comment 24 Johnny(Jianning) Ding 2010-10-20 17:59:49 PDT
(In reply to comment #23)
> Miss test exceptions in skip list for GTK/win ports. Will add it later.
I will move those two tests to another folder and send another patch for review.
Comment 25 Johnny(Jianning) Ding 2010-10-20 22:45:43 PDT
Created attachment 71391 [details]
patch v1 (with layout tests)
Comment 26 Adam Barth 2010-10-20 22:51:47 PDT
Comment on attachment 71391 [details]
patch v1 (with layout tests)

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

> WebCore/loader/NavigationScheduler.cpp:82
> +    // The derived class can call this method to override the m_wasUserGesture.
> +    void overrideWasUserGesture(bool wasUserGesture) { m_wasUserGesture = wasUserGesture; }

I'd remove this comment and call the function clearUserGesture().  Currently we don't have a use case for passing "true" here, and I'm not sure we will.

> WebCore/loader/NavigationScheduler.cpp:139
> +        // All ScheduledRedirects are not user-initiated, we need to override the m_wasUserGesture to false.
> +        overrideWasUserGesture(false);

I'd remove this comment too.  The test is a better explanation for why this line of code exists.

> LayoutTests/fast/frames/location-redirect-user-gesture.html:11
> +        setTimeout("location.href='data:text/html\,<script>layoutTestController.notifyDone()</" + "script>'", 1000);

The \ isn't needed.

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:11
> +    if (1 || window.layoutTestController) {

1 || window.layoutTestController will always be true.

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:17
> +        if (2 == sessionStorage.loadCount) {

Please clear session storage after this test so we can reload the test and have it work.

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:18
> +            layoutTestController.notifyDone();

Doesn't this race with the refresh?

I feel like we're being cheap.  Why not just add another file and refresh to that file?  Then there isn't a worry about an infinite loop.
Comment 27 Johnny(Jianning) Ding 2010-10-21 00:17:22 PDT
Created attachment 71396 [details]
patch v2

(In reply to comment #26)
All done.
> (From update of attachment 71391 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=71391&action=review
> ...
> 1 || window.layoutTestController will always be true.
forgot to remove the test code:-(
Comment 28 Adam Barth 2010-10-21 00:21:28 PDT
Comment on attachment 71396 [details]
patch v2

Thanks!
Comment 29 Pierre-Antoine LaFayette 2010-10-30 18:05:25 PDT
Hi Johnny, I've tested the meta refresh case with today's build of Chromium and I'm still seeing IsProcessingUserGesture as true for the second provisional load when clicking on the link.

These are the test files I'm using:

9044.html:
<html>
<head>
<title>Issue 9044</title>
</head>
<body>
<ul>
    <li>
    <a href="9044_meta_redirect.html">Meta redirect to file</a>
    <li>
</ul>
</body>
</html>

9044_meta_redirect.html:
<html>
<head>
<meta HTTP-EQUIV="REFRESH" content="0; url=http://cygwin.com/setup.exe">
</head>
<body>
</body>
</html>

Can you reproduce this? I'm assuming that your changes are in the current Chromium build but I'm not sure.
Comment 30 Johnny(Jianning) Ding 2010-10-30 19:22:28 PDT
(In reply to comment #29)
> Can you reproduce this? I'm assuming that your changes are in the current Chromium build but I'm not sure.

Unfortunately, the code still is not landed. Now I am not a committer, so I need someone who has committer privilege to land it. 
@Adam, would you please help me land it when you are free? Thanks!

@Pierre-Antoine, to test whether the issue can still be reproduced after landing this patch, I suggest that you download the latest patch and patch it under your own build/third_party/WebKit, then try again. Hope it helps:-)
Comment 31 Pierre-Antoine LaFayette 2010-10-30 19:52:59 PDT
Oh right! False alarm :) I've tested the patch and it works as advertised. I didn't notice it hadn't even hit the commit queue.

Thanks!
Comment 32 WebKit Commit Bot 2010-10-30 20:06:40 PDT
Comment on attachment 71396 [details]
patch v2

Rejecting patch 71396 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', 71396]" exit_code: 2
Last 500 characters of output:
tTests/fast/frames/meta-refresh-user-gesture.html
patching file LayoutTests/fast/frames/resources/meta-refresh-target.html
patching file LayoutTests/platform/gtk/Skipped
Hunk #1 FAILED at 5911.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/gtk/Skipped.rej
patching file LayoutTests/platform/win/Skipped
Hunk #1 succeeded at 1047 (offset 5 lines).

Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Adam Barth', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/4847096
Comment 33 Pierre-Antoine LaFayette 2010-11-07 01:26:55 PST
(In reply to comment #32)
> (From update of attachment 71396 [details])
> Rejecting patch 71396 from commit-queue.
> 
> Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', 'apply-attachment', '--force-clean', '--non-interactive', 71396]" exit_code: 2
> Last 500 characters of output:
> tTests/fast/frames/meta-refresh-user-gesture.html
> patching file LayoutTests/fast/frames/resources/meta-refresh-target.html
> patching file LayoutTests/platform/gtk/Skipped
> Hunk #1 FAILED at 5911.
> 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/gtk/Skipped.rej
> patching file LayoutTests/platform/win/Skipped
> Hunk #1 succeeded at 1047 (offset 5 lines).
> 
> Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Adam Barth', u'--force']" exit_code: 1
> 
> Full output: http://queues.webkit.org/results/4847096

Looks like the patch needs rebasing.
Comment 34 Alexey Proskuryakov 2010-11-07 20:59:13 PST
Comment on attachment 71396 [details]
patch v2

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

> LayoutTests/fast/frames/meta-refresh-user-gesture.html:13
> +In mete refresh redirection, the user gesture status in didStartProvisionalLoad callback should be false. For more details, please refer to https://bugs.webkit.org/show_bug.cgi?id=47817.

The bug doesn't really have more detail. It sort of kind of makes sense, but there is no concrete explanation or problem example in the bug.

 I don't know what's the use of checking for user gesture when downloading, so I can't tell what the benefit of making the indication more precise is.

> LayoutTests/platform/gtk/Skipped:5915
> +# https://bugs.webkit.org/show_bug.cgi?id=47849 and https://bugs.webkit.org/show_bug.cgi?id=47817

These are not correct bug numbers. Bug in Skipped list comment should be tracking a failure or missing DumpRenderTree feature. If there is no tracking bug, a comment should explain the failure.

> LayoutTests/platform/win/Skipped:1046
> +# https://bugs.webkit.org/show_bug.cgi?id=47849 and https://bugs.webkit.org/show_bug.cgi?id=47817

Ditto.
Comment 35 Johnny(Jianning) Ding 2010-11-08 10:10:57 PST
(In reply to comment #34)
Thanks, Alexey!
> The bug doesn't really have more detail. It sort of kind of makes sense, but there is no concrete explanation or problem example in the bug.
> 
>  I don't know what's the use of checking for user gesture when downloading, so I can't tell what the benefit of making the indication more precise is.

I thought Pierre-Antoine LaFayette's #1 comment might explain this issue. Anyway, I gonna change the explanation in test case to the following contents. @Pierre-Antoine LaFayette, can you also review it to see whether it explains this bug? Thanks!

Some WebKit ports send the user gesture to the embedders in didStartProvisionalLoad callback. The embedders use it to judge the navigation is user-initiated or not. In mete refresh redirection, the user gesture status in didStartProvisionalLoad callback should be false. 
For more details, please refer to https://bugs.webkit.org/show_bug.cgi?id=47817.

> These are not correct bug numbers. Bug in Skipped list comment should be tracking a failure or missing DumpRenderTree feature. If there is no tracking bug, a comment should explain the failure.

I change the comment to the following explanation.

# The following tests requires the DRT's dumpUserGestureInFrameLoadCallbacks
# method. But that method is not implemented in gtk port since gtk port can't
# get user gesture in frameload callbacks.
# The related bugs are https://bugs.webkit.org/show_bug.cgi?id=47849/47817.

Rebasing the patch, upload soon.
Comment 36 Johnny(Jianning) Ding 2010-11-09 07:06:05 PST
Created attachment 73373 [details]
re-baselined landing patch,
Comment 37 Eric Seidel (no email) 2010-11-10 14:00:55 PST
Comment on attachment 71396 [details]
patch v2

Cleared Adam Barth's review+ from obsolete attachment 71396 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 38 Pierre-Antoine LaFayette 2010-11-10 14:42:30 PST
There's a typo in LayoutTests/fast/frames/meta-refresh-user-gesture.html: 
In mete refresh redirection => In meta refresh redirection.
Comment 39 Johnny(Jianning) Ding 2010-11-11 04:36:18 PST
(In reply to comment #38)
> There's a typo in LayoutTests/fast/frames/meta-refresh-user-gesture.html: 
> In mete refresh redirection => In meta refresh redirection.

Thanks, after the patch (only re-baselined compared with last review+ patch) gets green light, I will ask for committer's help to fix the typo.
Comment 40 Pierre-Antoine LaFayette 2010-11-15 13:50:26 PST
(In reply to comment #39)
> (In reply to comment #38)
> > There's a typo in LayoutTests/fast/frames/meta-refresh-user-gesture.html: 
> > In mete refresh redirection => In meta refresh redirection.
> 
> Thanks, after the patch (only re-baselined compared with last review+ patch) gets green light, I will ask for committer's help to fix the typo.

The patch needs a new review since Eric cleared the review flag.
Comment 41 Eric Seidel (no email) 2010-11-15 15:00:04 PST
if abarth already reviewed, you can just fill him in as the reviewer in teh ChangeLog.  The clearning of flags on an obsolete patch is orthogonal to the review process.
Comment 42 Johnny(Jianning) Ding 2010-11-18 01:30:24 PST
(In reply to comment #41)
> if abarth already reviewed, you can just fill him in as the reviewer in teh ChangeLog.  The clearning of flags on an obsolete patch is orthogonal to the review process.

Thanks Eric! I re-generated the patch to resolve the latest conflict and filled Adam as reviewer in the ChangeLog. 
I guess we can land it patch now without a review again, am I right? If yes,  I want to ask for Abhishek's help to land it since Adam is kind of busy recently.
Comment 43 Johnny(Jianning) Ding 2010-11-18 01:31:37 PST
Created attachment 74215 [details]
re-baselined landing patch v2
Comment 44 Johnny(Jianning) Ding 2010-11-30 16:51:02 PST
Created attachment 75227 [details]
patch to resolve the latest conflict

Sorry for delay. After checking with Adam, I am gonna manually land it now
Comment 45 Johnny(Jianning) Ding 2010-11-30 16:58:10 PST
Created attachment 75230 [details]
Patch for landing
Comment 46 WebKit Commit Bot 2010-12-02 07:29:49 PST
The commit-queue encountered the following flaky tests while processing attachment 75230 [details]:

inspector/styles-new-API.html

Please file bugs against the tests.  These tests were authored by apavlov@chromium.org and pfeldman@chromium.org.  The commit-queue is continuing to process your patch.
Comment 47 WebKit Commit Bot 2010-12-02 07:31:41 PST
Comment on attachment 75230 [details]
Patch for landing

Rejecting patch 75230 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=abarth-cq-sl', 'land-attachment', '--force-clean', '--ignore-builders', '--non-interactive', '--parent-command=commit-queue', 75230]" exit_code: 2
Last 500 characters of output:
t-queue/WebCore/ChangeLog
Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1
Cannot dcommit with a dirty index.  Commit your changes first, or stash them with `git stash'.
 at /usr/local/git/libexec/git-core/git-svn line 485


Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1
Cannot dcommit with a dirty index.  Commit your changes first, or stash them with `git stash'.
 at /usr/local/git/libexec/git-core/git-svn line 485


Failed to run "[u'git', u'svn', u'dcommit']" exit_code: 1

Full output: http://queues.webkit.org/results/6837008
Comment 48 Eric Seidel (no email) 2010-12-02 09:28:53 PST
Comment on attachment 75230 [details]
Patch for landing

The second failure seems like a cq bug.  It may have hit an exception while trying to clean up things.
Comment 49 WebKit Commit Bot 2010-12-02 12:17:30 PST
Comment on attachment 75230 [details]
Patch for landing

Clearing flags on attachment: 75230

Committed r73181: <http://trac.webkit.org/changeset/73181>
Comment 50 WebKit Commit Bot 2010-12-02 12:17:38 PST
All reviewed patches have been landed.  Closing bug.
Comment 51 WebKit Commit Bot 2010-12-02 12:25:32 PST
The commit-queue encountered the following flaky tests while processing attachment 75230 [details]:

fast/media/mq-js-media-except-01.html
fast/preloader/script.html

Please file bugs against the tests.  These tests were authored by abarth@webkit.org and kimmo.t.kinnunen@nokia.com.  The commit-queue is continuing to process your patch.