Bug 103542 - [Chromium] Remove pluginsScriptableObject from PlatformSupport
Summary: [Chromium] Remove pluginsScriptableObject from PlatformSupport
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Pilgrim (Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-28 11:36 PST by Mark Pilgrim (Google)
Modified: 2012-11-28 14:40 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.53 KB, patch)
2012-11-28 11:37 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2012-11-28 12:10 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2012-11-28 12:42 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff
Patch (5.64 KB, patch)
2012-11-28 13:53 PST, Mark Pilgrim (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Pilgrim (Google) 2012-11-28 11:36:30 PST
[Chromium] Remove pluginsScriptableObject from PlatformSupport
Comment 1 Mark Pilgrim (Google) 2012-11-28 11:37:20 PST
Created attachment 176533 [details]
Patch
Comment 2 Adam Barth 2012-11-28 11:45:12 PST
Comment on attachment 176533 [details]
Patch

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

> Source/WebCore/bindings/v8/ScriptController.cpp:525
>      if (widget->isFrameView())
>          return 0;

in WebCore/bindings/js/ScriptController.cpp, the test is:

    if (!widget->isPluginView())
        return 0;

We should probably change this line here to:

    if (!widget->isPluginViewBase())
        return 0;

to ensure that the static_cast to PluginViewBase on like 527 is safe.

> Source/WebCore/plugins/PluginViewBase.h:35
> +#else

Sorry for steering you wrong on change.  We probably want to use a USE(JSC) ifdef for the JSC stuff rather than assuming that !USE(V8) is JSC.
Comment 3 Mark Pilgrim (Google) 2012-11-28 12:10:15 PST
Created attachment 176541 [details]
Patch
Comment 4 Mark Pilgrim (Google) 2012-11-28 12:10:45 PST
Comment on attachment 176541 [details]
Patch

Addressed review feedback.
Comment 5 Adam Barth 2012-11-28 12:36:14 PST
Comment on attachment 176541 [details]
Patch

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

> Source/WebCore/bindings/v8/ScriptController.cpp:524
> -    if (widget->isFrameView())
> +    if (widget->isPluginViewBase())

Don't we need a ! here?
Comment 6 Mark Pilgrim (Google) 2012-11-28 12:42:47 PST
Created attachment 176550 [details]
Patch
Comment 7 Mark Pilgrim (Google) 2012-11-28 12:43:23 PST
Comment on attachment 176550 [details]
Patch

Addressed latest feedback.
Comment 8 Adam Barth 2012-11-28 12:49:19 PST
Comment on attachment 176550 [details]
Patch

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

> Source/WebCore/plugins/PluginViewBase.h:33
> +#if USE(V8)

Technically this ifdef isn't needed since this declaration is the same for V8 and JSC, but we can leave it this way given that its only used for USE(V8).
Comment 9 WebKit Review Bot 2012-11-28 13:04:23 PST
Comment on attachment 176550 [details]
Patch

Rejecting attachment 176550 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
nt): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 cleanup: Change some code to be cleaner, more readable and style conforming

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 154.

Full output: http://queues.webkit.org/results/15018575
Comment 10 Mark Pilgrim (Google) 2012-11-28 13:53:08 PST
Created attachment 176567 [details]
Patch
Comment 11 Mark Pilgrim (Google) 2012-11-28 13:53:33 PST
Comment on attachment 176567 [details]
Patch

Resubmitted after commit failure.
Comment 12 WebKit Review Bot 2012-11-28 14:40:01 PST
Comment on attachment 176567 [details]
Patch

Clearing flags on attachment: 176567

Committed r136059: <http://trac.webkit.org/changeset/136059>
Comment 13 WebKit Review Bot 2012-11-28 14:40:06 PST
All reviewed patches have been landed.  Closing bug.