Bug 103542 – [Chromium] Remove pluginsScriptableObject from PlatformSupport

• Component: New Bugs
• Status: RESOLVED FIXED

[Mark Pilgrim (Google)]

[Chromium] Remove pluginsScriptableObject from PlatformSupport

[Mark Pilgrim (Google)]

Created attachment 176533 [details]
Patch

[Adam Barth]

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.

[Mark Pilgrim (Google)]

Created attachment 176541 [details]
Patch

[Mark Pilgrim (Google)]

Comment on attachment 176541 [details]
Patch

Addressed review feedback.

[Adam Barth]

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?

[Mark Pilgrim (Google)]

Created attachment 176550 [details]
Patch

[Mark Pilgrim (Google)]

Comment on attachment 176550 [details]
Patch

Addressed latest feedback.

[Adam Barth]

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).

[WebKit Review Bot]

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

[Mark Pilgrim (Google)]

Created attachment 176567 [details]
Patch

[Mark Pilgrim (Google)]

Comment on attachment 176567 [details]
Patch

Resubmitted after commit failure.

[WebKit Review Bot]

Comment on attachment 176567 [details]
Patch

Clearing flags on attachment: 176567

Committed r136059: <http://trac.webkit.org/changeset/136059>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.