Bug 35623

Summary: Page should not care about Chromium plug-in implementation details
Product: WebKit Reporter: Mark Rowe (bdash) <mrowe>
Component: Plug-insAssignee: Darin Fisher (:fishd, Google) <fishd>
Status: RESOLVED FIXED    
Severity: Normal CC: fishd
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
v1 patch mrowe: review+, fishd: commit-queue-

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