Closed Bug 660721 Opened 9 years ago Closed 9 years ago

move nsPluginInstanceOwner to its own files in dom/plugins/base/

Categories

(Core :: Plug-ins, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jaas, Assigned: jaas)

References

Details

Attachments

(2 files, 6 obsolete files)

We should move nsPluginInstanceOwner to its own files in dom/plugins/base/. This is part of bug 90268, after which nsObjectFrame will no longer be the instance owner.
OS: Mac OS X → All
Hardware: x86 → All
Blocks: 598425
Attached patch fix v1.0 (obsolete) — Splinter Review
You forgot to attach the new files.

You might get a nice diff if you hg copy nsObjectFrame.cpp to nsPluginInstanceOwner.cpp and then modify the latter.
Attached patch fix v1.1 (obsolete) — Splinter Review
Add new files. I've only compiled and run this on Mac OS X so far. Working on Linux now.
Attachment #536220 - Attachment is obsolete: true
Attached patch fix v1.2 (obsolete) — Splinter Review
Linux build fixes.
Attachment #536222 - Attachment is obsolete: true
Comment on attachment 536239 [details] [diff] [review]
fix v1.2

>+  PRInt32 blockPopups =
>+  Preferences::GetInt("privacy.popups.disable_from_plugins");

nit: fix the wrong indentation.
Attached patch fix v1.3 (obsolete) — Splinter Review
Attachment #536239 - Attachment is obsolete: true
Attachment #536307 - Flags: review?(roc)
This patch doesn't minimize the number of #includes in nsObjectFrame.cpp but we can take care of that in followup. This should be good enough for round one.
This patch doesn't do what I suggested in comment #2 ... any particular reason why?
Attached patch fix v1.4 (obsolete) — Splinter Review
QT build fixes.
Attachment #536307 - Attachment is obsolete: true
Attachment #536307 - Flags: review?(roc)
No reason, I just had it started a different way. I can do that if you want.
You should just be able to move aside your new file, "hg copy" nsObjectFrame.cpp over, then replace the copy with your new file. If we're lucky we'll get a nicer diff.
Attached patch fix v1.5Splinter Review
This is a patch produced by moving nsObjectFrame.cpp.
Attachment #536459 - Attachment is obsolete: true
Attachment #536501 - Flags: review?
Attachment #536501 - Flags: review? → review?(roc)
Comment on attachment 536501 [details] [diff] [review]
fix v1.5

Review of attachment 536501 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with that fixed

::: dom/plugins/base/Makefile.in
@@ +128,5 @@
>  endif
>  
>  LOCAL_INCLUDES = \
>    -I$(topsrcdir)/xpcom/base/ \
> +  $(MOZ_CAIRO_CFLAGS) \

This doesn't seem right, LOCAL_INCLUDES should just include -I options, right?

I know you just copied this, but why not add them to CXXFLAGS instead?
Comment on attachment 536501 [details] [diff] [review]
fix v1.5

Review of attachment 536501 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #536501 - Flags: review?(roc) → review+
I had originally put a bunch of the functions in a different order, the diff wasn't very nice until I put them back in the correct order. I did a bunch of that, not all of it though. Thanks for the review!
s/correct order/original order/
Attached patch fix v1.6 (obsolete) — Splinter Review
Makefile change that roc suggested.
Attached patch fix v1.7Splinter Review
Another QT build fix.
Attachment #536508 - Attachment is obsolete: true
pushed to mozilla-central

http://hg.mozilla.org/mozilla-central/rev/c4999efc5a84
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 670230
You need to log in before you can comment on or make changes to this bug.