Bug 35623 - Page should not care about Chromium plug-in implementation details
Summary: Page should not care about Chromium plug-in implementation details
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
: 35627 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-03-02 17:23 PST by Mark Rowe (bdash)
Modified: 2010-03-03 16:47 PST (History)
1 user (show)

See Also:


Attachments
v1 patch (1.86 KB, patch)
2010-03-03 12:47 PST, Darin Fisher (:fishd, Google)
mrowe: review+
fishd: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Rowe (bdash) 2010-03-02 17:23:55 PST
<http://trac.webkit.org/changeset/55443> added #if !PLATFORM(CHROMIUM) in the middle of a cross-platform block of code.  The build failure that it claimed to address was addressed on other platforms that don’t use PluginView by <http://trac.webkit.org/changeset/55440>, which doesn’t require Page to know about how each platform happens to implement plug-ins.  Chromium should follow this approach rather than adding #ifs in the manner of r55443.
Comment 1 Mark Rowe (bdash) 2010-03-02 20:17:45 PST
*** Bug 35627 has been marked as a duplicate of this bug. ***
Comment 2 Darin Fisher (:fishd, Google) 2010-03-02 20:20:04 PST
From bug 35627:
> One idea is to move the body of Page::privateBrowsingStateChanged to a
> static function on PluginView.  That function would be passed the Page,
> and it would then iterate over the list of PluginViews and notify each.
> 
> This way PluginViewNone.cpp can just have an empty body for that static
> method.

Sound good?
Comment 3 Darin Fisher (:fishd, Google) 2010-03-03 12:47:12 PST
Created attachment 49933 [details]
v1 patch

Just building PluginViewNone.cpp turns out to be sufficient.  Page::privateBrowsingStateChanged() is optimized away (tested using Visual Studio 8).
Comment 4 Darin Fisher (:fishd, Google) 2010-03-03 16:47:38 PST
Landed as http://trac.webkit.org/changeset/55486